git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 0/9] respect binary attribute in grep

From
Jeff King <peff@peff.net>
Date
Feb 4, 2012, 23:18 UTC
Message-ID
<20120204231825.GA1170@sigill.intra.peff.net>
In-Reply-To
<20120204192252.GA15319@padd.com>
On Sat, Feb 04, 2012 at 02:22:52PM -0500, Pete Wyckoff wrote:
> I took a look at this series.  It's nice.  My worry was that the
> extra open() of non-existent .gitattributes files in all the
> directories would cause performance problems across networked
> filesystems like NFS.

Yeah, I was able to measure a small slow-down on a quick grep even with a warm cache. So it does take some extra effort, but I think the correctness is worth it (and note that the slow down is in the tens of milliseconds if you have a reasonable stat()).

If people have big trees on NFS (or some other slow-stat system) where these lookups are actually a problem, I'd rather see a global option to disable .gitattributes lookups for both diff and grep (i.e., a "trust me, I'm not using gitattributes, and don't bother with stat" flag). In practice, though, I think such a thing is not necessary because the stat() is local to the file being examined (e.g., for "foo/bar/baz", we look only at "foo/bar/.gitattributes", "foo/.gitattributes", and ".gitattributes", without having to touch other parts of the tree).

Anyway, thanks for doing some performance testing. More data is always good.

Show 9 quoted lines
> It could be plausible that deep directory structures with few
> grep-able files will suffer with this change.  For example, many
> big binary blobs in deep directory hierarchies, but also some
> useful files here and there.
>
> One could argue that with the use of .gitattributes to specify
> which blobs should not be searched, this series makes this faster
> by not having to to read the binary blobs at all.  And I'd be
> okay with that.

Yes, exactly. I think this will end up being a big win for such cases, because the cost of loading even one large binary file from disk will dwarf all of the stats. But it does depend on people marking their binaries and using "-I".

-Peff
Previous: Pete WyckoffNext: Jeff King
Message 41 of 43 in “git-grep while excluding files in a blacklist”
  1. Dov GrobgeldJan 17, 2012
  2. Nguyen Thai Ngoc DuyJan 17, 2012
  3. Junio C HamanoJan 17, 2012
  4. Nguyen Thai Ngoc DuyJan 18, 2012
  5. Don't search files with an unset "grep" attributeconrad.irwin@gmail.com, Jan 23, 2012
  6. Junio C HamanoJan 23, 2012
  7. Don't search files with an unset "grep" attributeConrad Irwin, Jan 23, 2012
  8. Junio C HamanoJan 24, 2012
  9. Jeff KingJan 25, 2012
  10. Stephen BashJan 26, 2012
  11. Jeff KingJan 26, 2012
  12. Michael HaggertyJan 26, 2012
  13. Jeff KingJan 27, 2012
  14. Junio C HamanoFeb 1, 2012
  15. Jeff KingFeb 1, 2012
  16. Jeff KingFeb 1, 2012
  17. Conrad IrwinFeb 1, 2012
  18. Jeff KingFeb 1, 2012
  19. Jeff KingFeb 1, 2012
  20. Junio C HamanoFeb 2, 2012
  21. 1/2 grep: let grep_buffer callers specify a binary flagJeff King, Feb 1, 2012
  22. Junio C HamanoFeb 2, 2012
  23. Jeff KingFeb 2, 2012
  24. 0/9 respect binary attribute in grepJeff King, Feb 2, 2012
  25. 1/9 grep: make locking flag globalJeff King, Feb 2, 2012
  26. 2/9 grep: move sha1-reading mutex into low-level codeJeff King, Feb 2, 2012
  27. 3/9 grep: refactor the concept of "grep source" into an objectJeff King, Feb 2, 2012
  28. 4/9 convert git-grep to use grep_source interfaceJeff King, Feb 2, 2012
  29. 5/9 grep: drop grep_buffer's "name" parameterJeff King, Feb 2, 2012
  30. 6/9 grep: cache userdiff_driver in grep_sourceJeff King, Feb 2, 2012
  31. Junio C HamanoFeb 2, 2012
  32. Jeff KingFeb 2, 2012
  33. 7/9 grep: respect diff attributes for binary-nessJeff King, Feb 2, 2012
  34. 8/9 grep: load file data after checking binary-nessJeff King, Feb 2, 2012
  35. 9/9 grep: pre-load userdiff drivers when threadedJeff King, Feb 2, 2012
  36. Jeff KingFeb 2, 2012
  37. Thomas RastFeb 2, 2012
  38. Jeff KingFeb 2, 2012
  39. Junio C HamanoFeb 2, 2012
  40. Pete WyckoffFeb 4, 2012
  41. Jeff KingFeb 4, 2012
  42. 2/2 grep: respect diff attributes for binary-nessJeff King, Feb 1, 2012
  43. Junio C HamanoFeb 1, 2012

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.