Re: [PATCH 0/3] fast textconv

From: Jeff King <peff@peff.net>
Date: Mar 28, 2010, 16:56 UTC
Message-ID: <20100328165646.GA10293@coredump.intra.peff.net>
In-Reply-To: <20100328161921.GA3435@coredump.intra.peff.net>

On Sun, Mar 28, 2010 at 12:19:21PM -0400, Jeff King wrote:

Show 14 quoted lines

> On Sun, Mar 28, 2010 at 12:17:28PM -0400, Jeff King wrote:
> 
> > If I understand you right, you are proposing a separate program
> > that you would pass as a fasttextconv helper, and that would look in a
> > notes tree. So you would still have a per-diff fork/exec, and pipe all
> > the data.
> > 
> > I was thinking of actually doing it in-core, so cache hits would be as
> > lightweight as a notes lookup (and cache misses obviously would still
> > fork/exec a helper, but we don't care too much since the helper's time
> > to convert will dominate in that path).
> 
> Side note: I think I might prototype it as a separate program and see
> what kind of speed I can get.

Better, but not perfect. My script is below. I get:

  $ time git show >/dev/null
  real    0m1.036s
  user    0m0.412s
  sys     0m0.672s

which is still a 2.5x speedup (versus my other fast-textconv solution earlier in the thread), but I suspect we can do better. The notes mechanism does some up-front work to get very fast lookups, but because we invoke git-notes repeatedly, we never get the amortized benefit of that up-front work. Doing it in-core would fix that.

My script was:

-- >8 -- #!/bin/sh

type=$1; shift program=$1; shift sha1=$1; shift filename=$1; shift

GIT_NOTES_REF=refs/notes/textconv/$type; export GIT_NOTES_REF

# try the cache git notes show $sha1 2>/dev/null && exit 0

# otherwise, insert the cache entry. # We can be as slow as we like. ext=`echo "$filename" | sed 's/.*\.//'` tmp=`mktemp notes-textconv-XXXXXX.$ext` git cat-file blob $sha1 >$tmp $program $tmp | git notes add -f -F - $sha1 git notes show $sha1 rm -f $tmp -- 8< --

and my config is:

  $ git config diff.mfo.textconv /home/peff/fast mfo mfo-tags

where "mfo-tags" is the program to display metadata and "mfo" is a user-selected shorthand name for it.

-Peff