Re: ayjwork squid3/src HttpRequestMethod...

Alex Rousskov wrote:
> On Sun, 2008-02-03 at 12:11 +1300, Amos Jeffries wrote:
>
>> Alex, I'm droppign the conversion and not seeing many uses of METHOD_*
>> as an int. the ones present however look to be important a keepers.
>> You mentioned earlier when this class went it it might be an idea to
>> look at removing the _method_t enum entirely.
>
> I was thinking to make it private.

Aha, that would be different entirely.

>
>> After today I don't think it would be a good idea for performance.
>
> Please give an example or two, where the performance would noticeably
> suffer for standard methods.

There were several switch(e->method) in what I think is the request
processing pathways. I was thinking making that switch into a series of
string comparisons (on image()) would increase the processing ops where
the current index int checks are faster.
   src/client_side.cc:630 - clientIsContentLengthValid(HttpRequest * r)
   src/forward.cc:467 - checkRetriable()

I don't think it would be a _great_ sufferance, but it would be a small
unnecessary backslide. Making the enum 'methods which squid has special
internal processes for' would be simple and easier to maintain than
attempting to keep a comprehensive list with unexpected 'others' also
accepted silently.
And have the benefit of a low amount of change to the code for the same
gain.

The biggest possible hit I've seen so far with the enum change is the
store MD5's, they currently use the int value of the enum. I'm not sure
if it would matter making them always use the image() string instead.
They are certainly capable of it, just the side-effects need checking.
   src/store_key_md5.cc:120
   src/store_key_md5.cc:139

>
>> I'm thinking keeping GET, HEAD, POST, etc and any which are need for
>> fast path switching in the code.
>
> I doubt a switch-statement is noticeably faster than two or three "if
> method.isGet()" if-statements, but if you disagree, then let's keep
> those switch-statements in. We can still remove the auto-conversion
> operator because it does not improve performance and causes other
> problems.

Ah, no, not if we added special tests for them. I was thinking along the
line of the current API.

You are right one the speed of switch vs if, thats a code readability
preference.

Amos

-- 
Please use Squid 2.6STABLE17+ or 3.0STABLE1+
There are serious security advisories out on all earlier releases.