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.

Reply via email to