On Wed, Jul 31, 2013 at 01:26:16AM +0200, Branko Čibej wrote:
> On 30.07.2013 21:56, Greg Stein wrote:
> > It really doesn't make that much of a difference. APR_HASH_KEY_STRING
> > simply tells the hash function to watch for NUL rather than to
> > traverse N characters. Because N doesn't have to be tracked, you might
> > even argue the NUL-test is faster.
> >
> > IOW, passing APR_HASH_KEY_STRING does NOT perform a strlen(). So yes:
> > a lot of this work isn't saving much at all.
> >
> > Just look at apr/tables/apr_hash.c:hashfunc_default()
> >
> > Cheers,
> > -g
> >
> > (tho we'll leave it to Stefan and Daniel to decide whether to blow
> > more of their time on this "issue" or just back it all out)
>
> The commit that appears to cater to a new "requirement" of including
> svn_private_config.h before svn_hash.h frankly made my hair stand on
> end. Guys, do please take a couple steps back and look at the bigger
> picture. (Hint: it's a mess).
That could be easily fixed by moving SVN_HAS_DUNDER_ATOMICS to $CFLAGS.
(-DSVN_HAS_DUNDER_ATOMICS=0 on windows, and -DSVN_HAS_DUNDER_ATOMICS=`if
$svn_cv_dunder_builtins; then echo 1; else echo 0; fi` in configure.ac.)
I'll be happy to commit such a fix, assuming we don't rip out the optimization
altogether.
> My vote is to revert the whole shebang, possibly up to and including the
> introduction of svn_hash_sets/gets in the first place. Though that's in
> 1.8 now I believe...
Yes it is.
>
> P.S.: I don't recall anyone publishing any actual measurements that
> would confirm that the current code is faster. That's another hint.
I didn't check myself whether there was a performance gain when I implemented
this; I just saw people discussing *how* to implement it, so I joined the
discussion, assuming that the "whether" question has already been decided
(in the affirmative).
Perhaps someone else has performance figures?
Received on 2013-07-31 01:45:06 CEST