Re: ayjwork squid3/src HttpRequestMethod...

> On Sun, 2008-02-03 at 19:37 +1300, Amos Jeffries wrote:
>> >> 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
>> >
>> > Extension methods are not cachable, but we need to check whether
>> having
>> > the same id() or key() for multiple methods is going to create
>> problems.
>> > I doubt it will because all these store entries should remain private.
>>
>> Changing the MD5 affects both private and public (GET, HEAD, etc I
>> assume?) store MD5 functions.
>
> But we are not planning on changing anything in that regard, are we? We
> will keep passing method.id() to the key generator, right? My [mild]
> concern is that the id is now the same for all non-standard methods, but
> I hope that is not important as far as store is concerned because the
> entries will be private (i.e., one entry will never match others).

Ah, I see what your thinking now. yes, I'm not actually planning anything
that way. Just thinking about consequences to see if its worth considering
a plan.

As to your [mild] conscern. I think it would only affect collapsed
forwarding if anything. Which is not in squid-3 yet I believe.

>
>> > I am fine with keeping method_t exposed if hiding it creates too many
>> > problems. Let's just replace the conversion operator with id() and get
>> > rid of EXT values, I guess.
>>
>> The compile and unit-test fixes are done and a patch ready (attached) if
>> you want to test it.
>
> I will not have time to test it now and do not want to bother you with
> minor comments. Please commit when you feel comfortable.

Oh well. It's in now.

Built and unit-tested on Ubuntu and Debian. I'll start run-tests later
tonight.

Amos
Received on Sun Feb 03 2008 - 14:18:01 MST