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

Re: [PATCH v13 11/20] index-helper: use watchman to avoid refreshing index with lstat()

From
Duy Nguyen <pclouds@gmail.com>
Date
Jul 3, 2016, 12:28 UTC
Message-ID
<CACsJy8DvZgj0w8z8LdeuY8HdTTcEu7ABPyFi_wnmEZOGsoMkDQ@mail.gmail.com>
In-Reply-To
<loom.20160701T013515-311@post.gmane.org>
On Fri, Jul 1, 2016 at 1:54 AM, Ben Peart <peartben@gmail.com> wrote:
Show 5 quoted lines
> I've found (at least on Windows) that as the repo size gets larger, the
> time to read the index becomes a much smaller percentage of the overall
> time.  I just captured some perf traces of git status on a large repo we
> have.  Of that, 92.5% was spent in git!read_directory and only 4.0% was
> spent in git!read_index.  Of that 4%, 2.6% was git!glk_SHA1_Update.

OK.. Windows. I don't have Windows numbers, but "git!read_directory" (is that read_directory() in "git" process?) does not do a lot of stat'ing. Although I agree that it dominates git-status. I recorded some numbers in nd/untracked-cache topic (see 9e59724 (update-index: manually enable or disable untracked cache - 2015-03-08)).

First step would be enabling that because besides directory traversing, this code does a lot of string processing that's hopefully eliminated by untracked cache extension. I cut git-status' time in half with it. The side effect though, is that it creates a new flow of stat(), one per directory. It would be great if you could do some measurements with untracked cache on Windows and see if we get similar gain.

Show 5 quoted lines
> Given there were no dirty files, Watchman would have made a huge
> improvement in the overall time but index helper would have had
> relatively little impact.  We've noticed this same pattern in all our
> repos which is what is driving our interest in the Watchman model and
> also shows why index-helper is of less interest.

Assuming that untracked cache cuts git-status time by half on Windows as well, then index read time now takes a bigger percentage, 8%, where it starts to make sense to optimize it.

On a quiet repository, having watchman does not help so much because we already reduce the significant number of filesystem-related system calls. Yes there is lstat() and eliminating it may gain some more (with watchman) and it matters on a repo with lots of directories. You probably can just take these lstat out (I can help point out where) and see how much the gain is.

Assuming (blindly again) that removing lstat helps like 10% of read_index(), we would need to profile untracked cache code and see where what we can do next. There are still a lot of directory traversing there (except that it traverses the cache instead of filesystem) and maybe we can do something. But I haven't gotten that far.

> While the current design hides watchman behind index-helper, if we were
> to change that model so they were independent, we would be interested
> in doing it in such a way that provided some abstraction so that it
> could be replaced with another file watching daemon.

Frankly I'm not that interested in replacing another file watching daemon. My first attempt at this problem was "file-watcher" which was tied to Linux inotify system only and it would make sense to make it replaceable. But watchman supports OS X, Linux, FreeBSD and Windows now, not just Linux only as before, why another abstraction layer? You could even replace "watchman.exe" binary. As long as you produce the same json data, your new daemon should still work.

Tying index caching daemon and file watching daemon (let's avoid "watchman" for now) gives us a bonus. Because both git and the caching daemon know that they read the same index, we could answer the question "what files are dirty?" with "file number 1, 5, 9 in the index" instead of sending full paths and git has to make some more lookups to identify them. In this series we send it as a compressed bit map. Probably not a big deal in terms of real performance, but it feels nice to do lookups less.

The second reason is because watchman daemon alone does not provide enough information to reduce untracked cache's lstat() as much as possible. The current approach in this series is a naive one, which works for some cases, but not optimal (I'll get to that). We need a separate long-running daemon to maintain extra info to reduce lstat(). Because our target is watchman, it does not make sense to add yet another daemon besides index-helper to do this.

OK the optimal lstat() reduction thing. Right now, if any file in a directory is updated, the directory is invalidated in untracked cache and we need to traverse it to collect excluded files again. But it does not have to be that way. We don't care if any file is _updated_ because it will not change untracked cache output. We care about what files are _added_ or _deleted_. New files will need to be classified as either tracked, untracked or ignored. Deleted files may invalid either three file lists. Watchman cannot answer "what files are added or deleted since the point X in time" and I agree that it's not watchman's job (watchman issue 65). So we have to maintain some more info by ourselves, e.g. the list of files at any requested "clock". With that we can compare the file lists of two "clock"s and tell git what files are added or deleted.

-- 
Duy
Previous: Ben PeartNext: Ben Peart
Message 24 of 35 in “read-cache.c: fix constness of verify_hdr()”
  1. 01/20 read-cache.c: fix constness of verify_hdr()David Turner, Jun 26, 2016
  2. 04/20 index-helper: new daemon for caching index and related stuffDavid Turner, Jun 26, 2016
  3. Johannes SchindelinJun 30, 2016
  4. Duy NguyenJun 30, 2016
  5. Johannes SchindelinJul 2, 2016
  6. Duy NguyenJul 2, 2016
  7. 03/20 pkt-line: add gentle version of packet_writeDavid Turner, Jun 26, 2016
  8. 02/20 read-cache: allow to keep mmap'd memory after readingDavid Turner, Jun 26, 2016
  9. 06/20 daemonize(): set a flag before exiting the main processDavid Turner, Jun 26, 2016
  10. 05/20 index-helper: add --strictDavid Turner, Jun 26, 2016
  11. 18/20 index-helper: optionally automatically runDavid Turner, Jun 26, 2016
  12. 17/20 index-helper: autorun modeDavid Turner, Jun 26, 2016
  13. 15/20 index-helper: kill modeDavid Turner, Jun 26, 2016
  14. 14/20 watchman: add a config option to enable the extensionDavid Turner, Jun 26, 2016
  15. 13/20 unpack-trees: preserve index extensionsDavid Turner, Jun 26, 2016
  16. 16/20 index-helper: don't run if already runningDavid Turner, Jun 26, 2016
  17. 12/20 update-index: enable/disable watchman supportDavid Turner, Jun 26, 2016
  18. 09/20 read-cache: add watchman 'WAMA' extensionDavid Turner, Jun 26, 2016
  19. 10/20 watchman: support watchman to reduce index refresh costDavid Turner, Jun 26, 2016
  20. 11/20 index-helper: use watchman to avoid refreshing index with lstat()David Turner, Jun 26, 2016
  21. Ben PeartJun 30, 2016
  22. Duy NguyenJun 30, 2016
  23. Ben PeartJun 30, 2016
  24. Duy NguyenJul 3, 2016
  25. Ben PeartJul 6, 2016
  26. Duy NguyenJul 8, 2016
  27. Duy NguyenJul 8, 2016
  28. 07/20 index-helper: add --detachDavid Turner, Jun 26, 2016
  29. 08/20 index-helper: log warningsDavid Turner, Jun 26, 2016
  30. 19/20 trace: measure where the time is spent in the index-heavy operationsDavid Turner, Jun 26, 2016
  31. 21/20 unix-socket.c: add stub implementation when unix sockets are not supportedNguyễn Thái Ngọc Duy, Jun 26, 2016
  32. Johannes SchindelinJun 27, 2016
  33. Duy NguyenJun 27, 2016
  34. Johannes SchindelinJun 28, 2016
  35. David TurnerJun 27, 2016

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.