[v4,0/7] use fsmonitor data in git diff eliminating O(num_files) calls to lstat

Message

Derrick Stolee via GitGitGadget Oct. 20, 2020, 1:40 p.m. UTC

Credit to alexmv who made this commit back in Dec, 2017 when he was at dbx.
I've rebased it and am submitting it now.

With fsmonitor enabled, git diff currently lstats every file in the repo
This makes use of the fsmonitor extension to skip lstat() calls on files
that fsmonitor judged as unmodified.

I was able to do some testing with/without this change in a large in-house
repo (~ 400k files).

-----------------------------------------
(1) With fsmonitor enabled - on master of git (2.29.0)
-----------------------------------------
../git/bin-wrappers/git checkout HEAD~200
strace -c ../git/bin-wrappers/git diff

% time     seconds  usecs/call     calls    errors syscall
------ ----------- ----------- --------- --------- ----------------
 99.64    4.358994          10    446257         3 lstat
  0.12    0.005353           7       764       360 open

(A subsequent call)
strace -c ../git/bin-wrappers/git diff

% time     seconds  usecs/call     calls    errors syscall
------ ----------- ----------- --------- --------- ----------------
 99.84    4.380955          10    444904         3 lstat
  0.06    0.002564         135        19           munmap
...

-----------------------------------------
(2) With fsmonitor enabled - with my patch
-----------------------------------------
../git/bin-wrappers/git checkout HEAD~200
strace -c ../git/bin-wrappers/git diff

% time     seconds  usecs/call     calls    errors syscall
------ ----------- ----------- --------- --------- ----------------
 50.72    0.003090         163        19           munmap
 19.63    0.001196         598         2           futex
...
  0.00    0.000000           0         4         3 lstat


-----------------------------------------
(3) With fsmonitor disabled entirely
-----------------------------------------

% time     seconds  usecs/call     calls    errors syscall
------ ----------- ----------- --------- --------- ----------------
 98.52    0.277085       92362         3           futex
  0.27    0.000752           4       191        63 open
...
  0.14    0.000397           3       158         3 lstat

I was able to encode this into a perf test in one of the commits.

Changes since Patch Series V1

 * Add git diff -- <pathspec> to perf tests
 * improve readability of bitwise ops

Changes since Patch Series V2

 * Add git add to perf tests
 * Refactor perf fsmonitor to simplify / remove redundancy
 * Add linting to perf tests
 * Added git diff -- <pathspec> for various sized pathspecs
 * Confirmed that refresh_fsmonitor was always being called / added to
   commit message

Changes since Patch Series V3

 * Move perf test linting to Makefile in perf/ directory

Alex Vandiver (1):
  fsmonitor: use fsmonitor data in `git diff`

Nipunn Koorapati (6):
  t/perf/README: elaborate on output format
  t/perf/p7519-fsmonitor.sh: warm cache on first git status
  t/perf: add fsmonitor perf test for git diff
  perf lint: add make test-lint to perf tests
  p7519-fsmonitor: refactor to avoid code duplication
  p7519-fsmonitor: add a git add benchmark

 diff-lib.c                | 15 +++++-
 t/Makefile                |  7 +--
 t/perf/Makefile           |  5 +-
 t/perf/README             |  2 +
 t/perf/p3400-rebase.sh    |  6 +--
 t/perf/p7519-fsmonitor.sh | 96 ++++++++++++++++++++++-----------------
 6 files changed, 81 insertions(+), 50 deletions(-)


base-commit: d4a392452e292ff924e79ec8458611c0f679d6d4
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-756%2Fnipunn1313%2Fdiff_fsmon-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-756/nipunn1313/diff_fsmon-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/756

Range-diff vs v3:

 1:  cba03dd40b = 1:  cba03dd40b fsmonitor: use fsmonitor data in `git diff`
 2:  1c7876166f = 2:  1c7876166f t/perf/README: elaborate on output format
 3:  401f696c81 = 3:  401f696c81 t/perf/p7519-fsmonitor.sh: warm cache on first git status
 4:  b3ad8faac4 = 4:  b3ad8faac4 t/perf: add fsmonitor perf test for git diff
 5:  28c1e488bf ! 5:  b534cd137a perf lint: check test-lint-shell-syntax in perf tests
     @@ Metadata
      Author: Nipunn Koorapati <[e-mail]>
      
       ## Commit message ##
     -    perf lint: check test-lint-shell-syntax in perf tests
     +    perf lint: add make test-lint to perf tests
      
     -    Perf tests have some seq instead of test_seq. This
     -    runs the existing tests on the perf tests as well.
     +    Perf tests have not been linted for some time.
     +    They've grown some seq instead of test_seq. This
     +    runs the existing lints on the perf tests as well.
      
          Signed-off-by: Nipunn Koorapati <[e-mail]>
      
     @@ t/Makefile: CHAINLINTTMP_SQ = $(subst ','\'',$(CHAINLINTTMP))
       CHAINLINTTESTS = $(sort $(patsubst chainlint/%.test,%,$(wildcard chainlint/*.test)))
       CHAINLINT = sed -f chainlint.sed
       
     -@@ t/Makefile: test-lint-executable:
     +@@ t/Makefile: test-lint: test-lint-duplicates test-lint-executable test-lint-shell-syntax \
     + 	test-lint-filenames
     + 
     + test-lint-duplicates:
     +-	@dups=`echo $(T) | tr ' ' '\n' | sed 's/-.*//' | sort | uniq -d` && \
     ++	@dups=`echo $(T) $(TPERF) | tr ' ' '\n' | sed 's/-.*//' | sort | uniq -d` && \
     + 		test -z "$$dups" || { \
     + 		echo >&2 "duplicate test numbers:" $$dups; exit 1; }
     + 
     + test-lint-executable:
     +-	@bad=`for i in $(T); do test -x "$$i" || echo $$i; done` && \
     ++	@bad=`for i in $(T) $(TPERF); do test -x "$$i" || echo $$i; done` && \
     + 		test -z "$$bad" || { \
       		echo >&2 "non-executable tests:" $$bad; exit 1; }
       
       test-lint-shell-syntax:
     @@ t/Makefile: test-lint-executable:
       test-lint-filenames:
       	@# We do *not* pass a glob to ls-files but use grep instead, to catch
      
     + ## t/perf/Makefile ##
     +@@
     + -include ../../config.mak
     + export GIT_TEST_OPTIONS
     + 
     +-all: perf
     ++all: test-lint perf
     + 
     + perf: pre-clean
     + 	./run
     +@@ t/perf/Makefile: pre-clean:
     + clean:
     + 	rm -rf build "trash directory".* test-results
     + 
     ++test-lint:
     ++	$(MAKE) -C .. test-lint
     ++
     + .PHONY: all perf pre-clean clean
     +
       ## t/perf/p3400-rebase.sh ##
      @@ t/perf/p3400-rebase.sh: test_expect_success 'setup rebasing on top of a lot of changes' '
       	git checkout -f -B base &&
 6:  b38f2984f9 = 6:  3b20f4c76e p7519-fsmonitor: refactor to avoid code duplication
 7:  d392a523f2 = 7:  6f97439936 p7519-fsmonitor: add a git add benchmark