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

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.

>
> 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.
> >>
> >>
> >
Received on Tue Feb 22 2011 - 13:18:53 MST