Re: git grep --threads 12 --textconv is effectively single-threaded

From: Jeff King <peff@peff.net>
Date: Jul 8, 2020, 20:13 UTC
Message-ID: <20200708201353.GA2354599@coredump.intra.peff.net>
In-Reply-To: <CAMP9c5mpJ9_HvEBTiEQj=vocTdH6N9uXkpLKiE8+hFbAt9p5Ow@mail.gmail.com>

On Wed, Jul 08, 2020 at 12:40:49PM -0500, Zach Riggle wrote:

Show 5 quoted lines

> So I just need to
> 
> git -c "diff.c.cachetextconv=true" ...
> 
> And the cache should automagically work?

Yes, but there's a bit of subtlety with what you're grepping.

Try this example setup:

  git init -q repo
  cd repo
  echo content >file
  echo 'file diff=foo' >.gitattributes
  git add .
  git commit -m base
  git config diff.foo.textconv 'echo >&2 converting...; sleep 1; tr a-z A-Z <'

which should make it clear when the filter actually runs.

Now try this:

  echo >&2 "==> no textconv"
  git grep . file
  echo >&2 "==> with textconv"
  git grep --textconv . file
  echo >&2 "==> cached (one)"
  git -c diff.foo.cachetextconv=true grep --textconv . file
  echo >&2 "==> cached (two)"
  git -c diff.foo.cachetextconv=true grep --textconv . file

We'd expect the final one to say "file:CONTENT" but _without_ a "converting" line. But it still runs the textconv script!

The problem is that the cache uses the blob id of the source as its key. But since we're grepping files in the working directory, they don't have an object id at all. If we grep in the tree, it works as expected:

  $ git -c diff.foo.cachetextconv=true grep --textconv . HEAD -- file
  converting...
  HEAD:file:CONTENT
  $ git -c diff.foo.cachetextconv=true grep --textconv . HEAD -- file
  HEAD:file:CONTENT

All of this textconv stuff was originally designed for the diff infrastructure. Even when it's diffing files, if they're tracked in the repository the diff code will pull the oid from the index (assuming the file is stat-clean). But the grep code doesn't do that (and I doubt that it matters for anything at all except this textconv caching feature).

It's probably possible to teach the grep code to do the same check-in-the-index trick, but I'm not sure how complicated it would be.

-Peff