Stefan Fuhrmann wrote:
> Julian Foad wrote:
>>> URL:http://svn.apache.org/viewvc?rev=1333326&view=rev
>>> Introduce private API functions that wrap apr_hash_make_custom
>>> and return hash tables that are 2 to 4 times faster than the
>>> APR default.
>>
>> Would it be sensible to propose these (the hash-functions) for
>> inclusion in APR itself?
>
> Certainly. The question would be whether Apache is
> meant to run on CPUs without a decent MUL.
I don't understand why that question is relevant.
>> I suggest changing the names to
>>
>> svn_hash__make_compatible
>> svn_hash__make # faster than the 'compatible' one
>>
>> [...]
>
> The "fast" variant had a potential page overflow (segfault).
> Fixing that issue would have made it slower than the
> compatible version for the APR_HASH_KEY_STRING case.
> Because the latter constitutes the vast majority of uses
> within SVN, I remove the *_fast code in r1340590.
Great, it's better to just have one version.
>>> Modified: subversion/trunk/subversion/libsvn_subr/hash.c
>>> URL:http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/hash.c?rev=1333326&r1=1333325&r2=1333326&view=diff
>>> ==============================================================================
>>> --- subversion/trunk/subversion/libsvn_subr/hash.c (original)
>>> +++ subversion/trunk/subversion/libsvn_subr/hash.c Thu May 3 07:16:11 2012
>>> +/*** Optimized hash functions ***/
>>> +
>>> +/* Optimized version of apr_hashfunc_default. It assumes that the CPU has
>>> + * 32-bit multiplications with high throughput of at least 1 operation
>>> + * every 3 cycles. Latency is not an issue. Another optimization is a
>>> + * mildly unrolled main loop.
>>
>> Such specific details should at least refer to a specific version
>> of apr_hashfunc_default(). Perhaps also (for the "1 op per 3 cycles"
>> part, in particular) a specific system architecture or compiler.
>
> r1340601 explains why that is a reasonable assumption.
What's missing is a statement that this is an optimized version of
"apr_hashfunc_default in APR 1.4.5".
- Julian
Received on 2012-05-21 12:39:04 CEST