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?
