Re: [PATCH] improve hack in src/base/TextException.h

because it involves tricks to embed the same code again and again in
different compilation units to perform the caching. IMVHO it is not
even worth the time we have collectively spent in this discussion ;)

On 2/22/11, Tsantilas Christos <chtsanti_at_users.sourceforge.net> wrote:
> On 02/22/2011 03:18 PM, Tsantilas Christos wrote:
>> On 02/22/2011 02:08 PM, Kinkie wrote:
>>> Hi Christos,
>>> My question is, why use the cache at all? It seems quite complex for
>>> a little gain..
>>
>> You mean the FileNameHashCached method?
>> It is used to compute the hash of the file name to be used inside the
>> "Must" function. We may have multiple Must calls inside a simple
>> function. We are calling many times the "Must" function for each HTTP
>> requests comes through squid.
>> If we do not cache the result we are going to compute this hash on every
>> Must call. It is not normal.
>>
>> The FileNameHashCached is not complex, it is a very simple function. And
>> the gain is huge not little.
>
> Well, with a second view, I saw that the FileNameHashCached called only
> if the argument/condition passed to Must is not true (on failures etc).
> So yes it is not important. We are expecting to have a small number of
> such cases.
> Anyway I do not think it is so complex this part of code (just an if),
> why should we compute this value again and again?
>
>>
>>
>>>
>>> kinkie
>>> roaming
>>>
>>> Il giorno 22/feb/2011 11.54, "Tsantilas Christos"
>>> <chtsanti_at_users.sourceforge.net <mailto:chtsanti_at_users.sourceforge.net>>
>>> ha scritto:
>>> > Hi all,
>>> >
>>> > On 02/21/2011 08:31 PM, Alex Rousskov wrote:
>>> >> On 02/21/2011 12:27 AM, Kinkie wrote:
>>> >>>> Sorry, I am not sure I understand the above. Should I reverse
>>> r11236 and
>>> >>>> commit a casting fix? Or do you want be to post a patch that some
>>> Hudson
>>> >>>> job will pick up and test somehow first?
>>> >>>
>>> >>> Why?
>>> >>> Judging from the comment in there it's just meant to trick the
>>> >>> compiler into not (mis)understanding that the FileNameHash
>>> function is
>>> >>> unused, because in the end it's invoked via trickery.
>>> >>
>>> >> FileNameHash is _sometimes_ unused. The function is used in
>>> compilation
>>> >> units where certain macros are used and only in those compilation
>>> units.
>>> >>
>>> >>
>>> >>> IMVHO there are two clean ways out of this:
>>> >>> - use a compiler-dependent thing such as __attribute__((unused)) to
>>> >>> politely inform the compiler that sometimes the programmer knows more
>>> >>> about stuff than the compiler
>>> >>
>>> >> A compiler-specific attribute is not a "clean way", especially when it
>>> >> is introduced as a fix for a rather different problem and actually
>>> kind
>>> >> of lies to the compiler in some cases.
>>> >>
>>> >>
>>> >>> - just get rid of this whole FileNameHashCached thing as a premature
>>> >>> optimization, and just recalculate the filename hash when we throw a
>>> >>> TextException (which is hopefully not that often)
>>> >>
>>> >> Sure, let's revisit the necessity of this hash caching optimization.
>>> >>
>>> >> Christos, do you remember what were the key reasons for adding it
>>> in the
>>> >> first place?
>>> >
>>> >
>>> > OK, with a little research I found it :-)
>>> > This is a part of the log error details patch.
>>> >
>>> > The original comment was:
>>> >> ... For the files the Must is not used, I got a compiler error that
>>> " the FileNameHashCached defined but not used"
>>> >> To solve it I just define a foo_class....
>>> >
>>> >
>>> > I am not sure about the "__attribute__((unused))" definition but if it
>>> > is only suppressing the warning, it is not bad idea...
>>> >
>>> >>
>>> >>
>>> >> Thank you,
>>> >>
>>> >> Alex.
>>> >>
>>> >>
>>> >
>>
>>
>
>

-- 
    /kinkie