[PATCH 2/2] grep: respect diff attributes for binary-ness

There is currently no way for users to tell git-grep that a particular path is or is not a binary file; instead, grep always relies on its auto-detection (or the user specifying "-a" to treat all binary-looking files like text).

This patch teaches git-grep to use the same attribute lookup that is used by git-diff. We could add a new "grep" flag, but that is unnecessarily complex and unlikely to be useful. Despite the name, the "-diff" attribute (or "diff=foo" and the associated diff.foo.binary config option) are really about describing the contents of the path. It's simply historical that diff was the only thing that cared about these attributes in the past.

And if this simple approach turns out to be insufficient, we still have a backwards-compatible path forward: we can add a separate "grep" attribute, and fall back to respecting "diff" if it is unset.

There are a few things worth nothing about the implementation.

One is that the attribute lookup happens outside of the grep.[ch] interface (i.e., outside of grep_buffer). We could do it at a lower level, which would be slightly more convenient for callers. However, this interacts badly with threading. The attribute-lookup code performs best when lookup order matches the filesystem order (so looking up "a/b/c" is cheaper if we just looked up "a/b/d" than if we just did "x/y/z"). Because we issue many simultaneous requests to grep_buffer, performing the attribute lookup at that level will cause requests from unrelated paths to interleave, and we lose the locality that makes the lookup optimization work.

Instead, in the threaded case we check the attributes as they are added to the work queue, meaning they are looked up in the optimal order (in the single threaded case, this is a non-issue, as we process the files serially in the optimal order).

Here are a few numbers showing the difference. The first is a best-of-five time for "git grep foo" on the linux-2.6 repo before this patch (on a 4-core HT processor, using 8 threads):

  real    0m0.306s
  user    0m1.512s
  sys     0m0.412s

Now here's the time for the same operation with a trial implementation looking up attributes in grep_buffer, showing a 31% slowdown:

  real    0m0.401s
  user    0m1.760s
  sys     0m0.636s

And here's the same operation with this patch, with only an 11% slowdown:

  real    0m0.339s
  user    0m1.444s
  sys     0m0.584s

Note that while the percentages are big, the absolute numbers are pretty small. In particular, this is a very inexpensive grep to do. A more complex regex should have the same absolute slowdown from the attribute lookup, but it would be a much smaller percentage of the total processing time. So doing it this way is not a huge win, but it does help on small greps.

The second issue worth noting is that while we do a full attribute lookup, we pass along only the binary flag to grep_buffer. When the "-p" flag is given to grep, we will actually look up the same attributes to find funcname patterns of matches. We could pass along the pointer to the userdiff driver for reuse.

However, it's not worth doing this. It clutters the code, as the driver has to be passed through a large number of helper functions (and pollutes the grep_buffer interface with userdiff code). And in my tests, it didn't actually improve performance. Because we only have to look up the attribute for a grep hit, in most cases we will only do the funcname lookup for a small subset of files. The cost of the extra lookups turns out to be negligible.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/grep.c         |   26 ++++++++++++++++++++++----
 t/t7008-grep-binary.sh |   24 ++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 4 deletions(-)