Re: [PATCH] Optimize HttpVersion comparison

On 08/23/2010 06:22 PM, Kinkie wrote:
> On Tue, Aug 24, 2010 at 1:52 AM, Alex Rousskov
> <rousskov_at_measurement-factory.com> wrote:
>> It is easy to get HTTP version comparison wrong. I have added a few
>> comparison operators. To be efficient, they need to be inlined by the
>> compiler so that the following expression does not have to cause
>> HttpVersion(1,0) creation and destruction:
>>
>> if (request->http_ver<= HttpVersion(1, 0))
>>
>> I tried to simplify the constructors and the quality comparison operator to
>> increase our chances that they will be inlined. Unfortunately, I ran into a
>> GCC "feature" which probably explains why we have not written the
>> HttpVersion constructor correctly before.
>>
>> I added a workaround. Should those #undef lines be moved to some
>> compatibility file? If yes, which one?
>
> Can't we just name our data fields Major and Minor?

Those specific names would violate member naming policy, but we can find
other names, of course (e.g., majr and minr). It would require changing
caller code which I wanted to avoid though.

> Also, would adding the inline keyword hint the compiler about our
> wishes? I guess it wouldn't hurt (it would also probably not help
> either, but still..)

It might, but only if the method definition is separated from the
declaration. In our case, all methods are defined and declared
simultaneously. I have not checked whether that is enough for GCC to
inline everything though.

> Apart from that, +1.

I am not going to commit this optimization until there is consensus on
how to handle the major/minor naming conflict with GCC.

Thank you,

Alex.
Received on Tue Aug 24 2010 - 03:44:22 MDT