Re: [PATCH 3/2] Avoid unnecessary strlen() calls

From: Shawn O. Pearce <spearce@spearce.org>
Date: Mar 20, 2007, 03:29 UTC
Message-ID: <20070320032947.GA29145@spearce.org>
In-Reply-To: <Pine.LNX.4.63.0703200400230.22628@wbgn013.biozentrum.uni-wuerzburg.de>

Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

Show 28 quoted lines

> On Sun, 18 Mar 2007, Linus Torvalds wrote:
> 
> > All the top profiling hits are about generating the patches and 
> > assigning blame:
> > 
> > 	samples  %        image name               app name                 symbol name
> > 	470352   15.5813  git                      git                      xdl_hash_record
> 
> I felt a little left out in all that performance slashing, and so I 
> thought maybe, just maybe, a small change in xdl_hash_record() can do 
> wonders (since it _is_ really simple, but still takes almost a 6th of the 
> CPU time). I don't have a proper test case setup, so maybe you want to try 
> this:
> 
> -- snipsnap --
> [PATCH] xdiff/xutils.c(xdl_hash_record): factor out whitespace handling
> 
> Since in at least one use case, xdl_hash_record() takes over 15% of the
> CPU time, it makes sense to even micro-optimize it. For many cases, no
> whitespace special handling is needed, and in these cases we should not
> even bother to check for whitespace in _every_ iteration of the loop.
> 
> Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> 
> ---
> 
> 	Please do not consider this patch _unless_ it is proven to enhance 
> 	the profile statistics substantially.

This is a massive difference for me. I ran it on git-gui.sh in the git-gui repository - this is a 6000 line file that has a lot of revisions, and has been renamed a few times. I applied the patch on top of current 'master' (v1.5.1-rc1), so I was testing with Linus' delta_base_cache.

# stock v1.5.1-rc1
$ for a in 1 2 3 4 5;do /usr/bin/time ../lt-blame blame --incremental HEAD git-gui.sh >/dev/null;done
        6.27 real         5.31 user         0.55 sys
        6.40 real         5.32 user         0.55 sys
        6.33 real         5.33 user         0.53 sys
        6.67 real         5.32 user         0.55 sys
        6.18 real         5.31 user         0.53 sys

# with the above patch
$ for a in 1 2 3 4 5;do /usr/bin/time ../js-blame blame --incremental HEAD git-gui.sh >/dev/null;done
        3.57 real         2.87 user         0.51 sys
        3.58 real         2.87 user         0.51 sys
        3.53 real         2.86 user         0.52 sys
        3.61 real         2.86 user         0.51 sys
        3.64 real         2.87 user         0.52 sys

For the record, both versions did produce identical output.

Given how small of a change it is, and how much of an improvement it made, I say apply it.

-- 
Shawn.