[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: Branko ─îibej <brane_at_apache.org>
Date: Wed, 23 May 2012 06:44:28 +0200

On 22.05.2012 21:01, Stefan Fuhrmann wrote:
> Am 21.05.2012 12:38, schrieb Julian Foad:
>> 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.
>
> APRs implementation uses 33 as multiplier which
> can conveniently be implemented as shift & add.
> My code uses factors up to 33^4 where that
> optimization / workaround would no longer be
> useful. A non-pipelined MUL operation may take
> as much as 40 ticks (i386) instead of 2 .. 6 ticks for
> shift&add.

I'd really like to see you explain why this change of yours (33 -> 33^4)
is relevant in practice. It's not at all clear that this multiplier
gives a better key distribution than the time-honoured 33.

It's my considered opinion that this fiddling around with hash function
implementations is way overboard. Just use apr_hashfunc_default already.
Unless you can prove that using your "optimized" version results in
siginificant savings in space and/or time, anything else is just piling
on more lines of code that need to be maintained for no good reason.

-- Brane
Received on 2012-05-23 06:44:43 CEST

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