[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1333326 - in /subversion/trunk/subversion: include/private/svn_hash_private.h libsvn_fs_fs/temp_serializer.c libsvn_subr/hash.c

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Mon, 21 May 2012 11:38:25 +0100 (BST)

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

This is an archived mail posted to the Subversion Dev mailing list.