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

 On Tue, 22 Feb 2011 10:06:59 -0700, Alex Rousskov wrote:
> On 02/22/2011 08:36 AM, Tsantilas Christos wrote:
>> On 02/22/2011 04:47 PM, Kinkie wrote:
>>> because it involves tricks to embed the same code again and again
>>> in
>>> different compilation units to perform the caching.
>
>> Well, I do not have strong opinion on that, we can remove caching if
>> we do not like. But this is a problem which it is solved and I doubt
>> that it can cause more problems. If we believe that every time we
>> are
>> going to add a new module or feature we have to spend time for a new
>> solution/hack to fix it, then we should remove it. This is just my
>> opinion.
>
> I may have been the one who suggested this optimization, but I am not
> strongly attached to it, and I am not against removing it if it
> causes
> more problems than it solves.
>
> I do not think adding a few bytes to every .o file to store the
> computed
> hash value is a problem, but others may find bigger flaws with the
> approach, of course.
>
> Perhaps others can vote and break the current tie? Current code
> caches
> the source code file name hash to avoid recomputing it for every
> failed
> Must(). Must()s do not fail often, but, unlike assert()s, they may
> fail
> when there are I/O and compatibility failures. The cost of such
> caching
> is a few extra bytes added to every .o file that uses TextException.
> The
> cost of not caching is computing a simple hash for a 10-40 byte
> string
> (depending on the source file and the compiler).

 FWIW: The argument that pushed me from "dont care" to "may as well do
 it" last time. Was that on high performance systems (only) the caching
 method is faster and the new code using Must() was being relied on in
 areas like adaptation so it *would* cause them performance hits.

 AFAICT the rest of the code is explicit in its failure handling so this
 does not matter there in the slightest.

 IMO, its three hacks fighting it out. None of them good, but our goal
 it to get certain compilers to STFU without letting it produce bad code.
  So far __attribute__ appears to be the smallest since it does not add
 extra bytes on top of the calculation function OR extra work to the .o.
 The non-GCC appear not to care in the slightest, which is telling since
 they are so much more picky with code style.

>
>>> IMVHO it is not
>>> even worth the time we have collectively spent in this discussion
>>> ;)

 Amen, Hallelujah, and anyone got a big enough rug to sweep this under?
 ;P

>
> That is probably true, at least short-term. FWIW, I would not bother
> keeping this pandora box open if it were not for the GCC-specific
> __attribute__ that was added by those who opened it :-).

 Answer me this: GCC specific complaint, why not a GCC specific fix?

 Amos
Received on Tue Feb 22 2011 - 21:22:21 MST