[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: Stefan Fuhrmann <eqfox_at_web.de>
Date: Sun, 20 May 2012 00:35:58 +0200

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.
>> Both yield repeatable results (each instance will store items in the same
>> order if the keys are the same). The first, svn_hask__make will return
> s/hask/hash/
Thanks for fixing!
>> a hash table that behaves like pre APR 1.4.6 default hashes.
>> * subversion/include/private/svn_hash_private.h
>> (svn_hash__make, svn_hash__make_fast): new private API
> I suggest changing the names to
> svn_hash__make_compatible
> svn_hash__make # faster than the 'compatible' one
> because being compatible is a specific requirement that some callers have (that might compromise speed or other properties), so we make this property explicit, whereas being "fast" is a vague and relative term that no caller specifically needs or doesn't need, it's just generally good for any function to be as fast as possible within its specific constraints.
> If we still have code that depends on the original hash ordering, then we wouldn't be able to drop in 'svn_hash__make' (the non-compatible version) as a direct replacement for apr_hash_make() throughout the code base -- but I don't know that we do have any such dependencies.
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.

>> 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.
>> + */
>> +static unsigned int
>> +hashfunc_compatible(const char *char_key, apr_ssize_t *klen)
> - Julian
Thanks for the review!

-- Stefan^2.
Received on 2012-05-20 02:36:48 CEST

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