Re: [PATCH] log error details

On 10/14/2010 04:28 AM, Tsantilas Christos wrote:
> On 10/14/2010 12:46 PM, Amos Jeffries wrote:
>> On 14/10/10 22:04, Tsantilas Christos wrote:
>>> On 10/14/2010 12:58 AM, Amos Jeffries wrote:

Plan A: the posted patch (compute runtime, roughly once per file).
Plan B: compute runtime in TextException, all the time.
Plan C: hard-coded constant hash value in every .h and .cc file.

>>>>>> The singleton hack making local copies in almost every file .o is
>>>>>> likely to symbol clash on the more picky compilers like MacOSX.

Possible, but not very likely because it would be a C++ standard
violation and because we probably already have identical static symbols
in many .o files. We can test though and revert to Plan B or C if there
are indeed clashes.

>>>>> We need a copy of the FileNameHashCached in each .o file, which will
>>>>> compute and cache once the hash of its filename. Alternatively we can
>>>>> compute the hash when the Must exception called (direct use of the
>>>>> FileNameHash).

>>>> IMO that would be better. A system throwing and failing a lot of Must()
>>>> requirements has worse problems than a few cycles of hashing.

>>> Probably you are right here. In the other hand some thousands of
>>> transactions aborted the same time for the same reason, will cause squid
>>> to compute the same hash value some thousand times.
>>>
>>> If we decide to not cache the hash value and there is not any objection
>>> from others developers I will do the followings:
>>> - remove the FileNaneHashCached and the FileNameHashCached class.
>>> - make FilenameHash a private method of the TextException class.
>>> - Compute the hash only in TextException::id() method. This will compute
>>> the hash value only if it is requested, not for all Must()

Let's call this Plan B.

>>>> * NP: if performance is _that_ much of a problem we had best look into
>>>> pre-calculating FileNameHash() results as a const parameter to the
>>>> Must/TextcHere macros during build.
>>> I can not find any simple way for this one. Any hint? But if we decide
>>> to not cache the hash value of the file name we do not need it.
>>
>> A separate script that runs and does the filename hashing. It might be
>> run by configure, top level Makefile, or if it was useful enough we
>> could add it to source-maitenance.sh for permanent embedding.

This is Plan C.

I do not think configure and similar build-time tools would work well
here. We do not want to work with foo.cc.in files all the time :-). I
agree that hard-coded hashes can be automated as a part of SourceFormat
scripts.

The biggest drawback of this approach is that it requires wrapping every
header file in push/redefine/pop macros (or at least redefine macros
after all the #include lines). Nevertheless it is doable.

> Also using a 16bit hash can produce the same hash value for two files,
> but it is not a real problem because it is unusual to have two files
> with the same hash value and a Must expression in the same line.

If the hash is automated and hard-coded by a script, the script can even
make sure there are no duplicates, I think.

I do not have a strong preference, but if nobody has a strong
preference, then I would recommend this order of commits/backtracking if
we find actual problems:

   Plan A -- because it is already implemented.
   Plan B -- because it is the simplest.
   Plan C -- because it is "fast" and eliminates dupes.

HTH,

Alex.
Received on Thu Oct 14 2010 - 18:16:54 MDT