* Dmitry Potapov <dpotapov@xxxxxxxxx> wrote:
> 2011/4/28 Ingo Molnar <mingo@xxxxxxx>:
> > +static inline int hashcmp(const unsigned char *sha1, const unsigned char *sha2)
> > {
> > - return !memcmp(sha1, null_sha1, 20);
> > + int i;
> > +
> > + for (i = 0; i < 20; i++, sha1++, sha2++) {
> > + if (*sha1 != *sha2) {
>
> At the very least, you may want to put 'likely' in this 'if'
> condition, otherwise the compiler may optimize this loop in
> the same way as with memcmp. So, it may work well now, but
> it may not work much slower with future versions or different
> level of optimization. (AFAIK, -O3 is far more aggressive in
> optimizing of loops).
the main difference is between the string assembly instructions and the loop.
Modern CPUs will hardly notice this loop being emitted with slight variations
by the compiler. So i do not share this concern.
> Another thing is that so far this optimization was only with
> GCC, and we do not know whether it helps or harms to compilers.
> So, maybe placing this code under 'ifdef __GNUC__' makes more
> sense than pushing this change on other compilers too.
Well, given that 90%+ of Git development is done with GCC (and probably the
user share is similarly high as well):
aldebaran:~/git> git log --pretty=oneline -i --grep gcc | wc -l
127
aldebaran:~/git> git log --pretty=oneline -i --grep llvm | wc -l
1
and given that these two patches speed up 'git gc' by 30%+, i doubt we'd want
to litter core Git code with #ifdef __GNUC__ uglinesses.
Thanks,
Ingo
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html