Julian Foad wrote:
>> 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
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 # 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
>> --- 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!
Received on 2012-05-20 02:36:48 CEST