Update fsmonitor perf suite to support integration comparisons #772
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In preparation for testing multiple fsmonitor hooks Signed-off-by: Nipunn Koorapati <nipunn@dropbox.com>
It is only required to be set up once. This prepares for testing multiple hooks in one invocation. Signed-off-by: Nipunn Koorapati <nipunn@dropbox.com>
Previously - it would silently run the perf suite w/o using fsmonitor - fsmonitor errors are not hard failures. Now it errors loudly. GIT_PERF_7519_FSMONITOR="$HOME/rs-git-fsmonitorr" ./p7519-fsmonitor.sh -i -v fatal: cannot run /home/nipunn/rs-git-fsmonitorr: No such file or directory not ok 2 - setup for fsmonitor Signed-off-by: Nipunn Koorapati <nipunn@dropbox.com>
There was much duplication here. Prepares for making changes to the description. Signed-off-by: Nipunn Koorapati <nipunn@dropbox.com>
The full name is lengthy and makes it hard to read Before: 7519.3: status (fsmonitor=/home/nipunn/src/server/.git/hooks/rs-git-fsmonitor) 0.02(0.01+0.00) After 7519.3: status (fsmonitor=rs-git-fsmonitor) 0.03(0.02+0.00) Signed-off-by: Nipunn Koorapati <nipunn@dropbox.com>
It is extremely verbose, printing >10K non-useful lines Signed-off-by: Nipunn Koorapati <nipunn@dropbox.com>
This prepares for it being called multiple times when testing different hooks Signed-off-by: Nipunn Koorapati <nipunn@dropbox.com>
Previously, the git add of the previous suiterun would pollute the numbers in the second run Before: Test this tree ----------------------------------------------------------------------------- 7519.4: status (fsmonitor=fsmonitor-watchman) 0.40(0.36+0.04) 7519.5: status -uno (fsmonitor=fsmonitor-watchman) 0.19(0.12+0.07) 7519.6: status -uall (fsmonitor=fsmonitor-watchman) 1.36(0.74+0.61) 7519.7: diff (fsmonitor=fsmonitor-watchman) 0.14(0.10+0.04) 7519.8: diff -- 0_files (fsmonitor=fsmonitor-watchman) 0.14(0.10+0.04) 7519.9: diff -- 10_files (fsmonitor=fsmonitor-watchman) 0.14(0.09+0.05) 7519.10: diff -- 100_files (fsmonitor=fsmonitor-watchman) 0.14(0.10+0.04) 7519.11: diff -- 1000_files (fsmonitor=fsmonitor-watchman) 0.14(0.08+0.06) 7519.12: diff -- 10000_files (fsmonitor=fsmonitor-watchman) 0.14(0.10+0.04) 7519.13: add (fsmonitor=fsmonitor-watchman) 2.03(1.28+0.69) 7519.16: status (fsmonitor=disabled) 0.64(0.49+0.90) 7519.17: status -uno (fsmonitor=disabled) 1.15(0.92+1.00) 7519.18: status -uall (fsmonitor=disabled) 2.32(1.46+1.55) 7519.19: diff (fsmonitor=disabled) 1.44(1.12+1.76) 7519.20: diff -- 0_files (fsmonitor=disabled) 0.11(0.07+0.05) 7519.21: diff -- 10_files (fsmonitor=disabled) 0.11(0.06+0.05) 7519.22: diff -- 100_files (fsmonitor=disabled) 0.11(0.08+0.03) 7519.23: diff -- 1000_files (fsmonitor=disabled) 0.11(0.08+0.04) 7519.24: diff -- 10000_files (fsmonitor=disabled) 0.12(0.06+0.07) 7519.25: add (fsmonitor=disabled) 2.25(1.47+1.47) After: Test this tree ----------------------------------------------------------------------------- 7519.4: status (fsmonitor=fsmonitor-watchman) 0.41(0.33+0.09) 7519.5: status -uno (fsmonitor=fsmonitor-watchman) 0.20(0.14+0.07) 7519.6: status -uall (fsmonitor=fsmonitor-watchman) 1.37(0.78+0.58) 7519.7: diff (fsmonitor=fsmonitor-watchman) 0.14(0.10+0.04) 7519.8: diff -- 0_files (fsmonitor=fsmonitor-watchman) 0.14(0.08+0.06) 7519.9: diff -- 10_files (fsmonitor=fsmonitor-watchman) 0.14(0.09+0.05) 7519.10: diff -- 100_files (fsmonitor=fsmonitor-watchman) 0.14(0.10+0.05) 7519.11: diff -- 1000_files (fsmonitor=fsmonitor-watchman) 0.14(0.11+0.04) 7519.12: diff -- 10000_files (fsmonitor=fsmonitor-watchman) 0.14(0.09+0.05) 7519.13: add (fsmonitor=fsmonitor-watchman) 2.04(1.27+0.71) 7519.16: status (fsmonitor=disabled) 0.78(0.59+0.99) 7519.17: status -uno (fsmonitor=disabled) 0.43(0.32+0.88) 7519.18: status -uall (fsmonitor=disabled) 1.58(0.96+1.38) 7519.19: diff (fsmonitor=disabled) 0.36(0.31+0.79) 7519.20: diff -- 0_files (fsmonitor=disabled) 0.11(0.08+0.03) 7519.21: diff -- 10_files (fsmonitor=disabled) 0.11(0.07+0.04) 7519.22: diff -- 100_files (fsmonitor=disabled) 0.11(0.08+0.04) 7519.23: diff -- 1000_files (fsmonitor=disabled) 0.11(0.07+0.05) 7519.24: diff -- 10000_files (fsmonitor=disabled) 0.12(0.08+0.05) 7519.25: add (fsmonitor=disabled) 2.25(1.48+1.47) Signed-off-by: Nipunn Koorapati <nipunn@dropbox.com>
Allows for simple perf comparison of different integrations. I ran it to compare our perl script w/ rs-git-fsmonitor and found 20-30ms of overhead on every command. Output looks like this (extra newlines added for readability) Test this tree --------------------------------------------------------------------------- 7519.4: status (fsmonitor=query-watchman) 0.42(0.37+0.05) 7519.5: status -uno (fsmonitor=query-watchman) 0.19(0.12+0.07) 7519.6: status -uall (fsmonitor=query-watchman) 1.36(0.73+0.62) 7519.7: diff (fsmonitor=query-watchman) 0.14(0.09+0.05) 7519.8: diff -- 0_files (fsmonitor=query-watchman) 0.14(0.11+0.03) 7519.9: diff -- 10_files (fsmonitor=query-watchman) 0.14(0.10+0.04) 7519.10: diff -- 100_files (fsmonitor=query-watchman) 0.14(0.09+0.05) 7519.11: diff -- 1000_files (fsmonitor=query-watchman) 0.14(0.08+0.06) 7519.12: diff -- 10000_files (fsmonitor=query-watchman) 0.14(0.09+0.05) 7519.13: add (fsmonitor=query-watchman) 2.04(1.32+0.66) 7519.16: status (fsmonitor=rs-git-fsmonitor) 0.39(0.32+0.08) 7519.17: status -uno (fsmonitor=rs-git-fsmonitor) 0.17(0.11+0.06) 7519.18: status -uall (fsmonitor=rs-git-fsmonitor) 1.33(0.71+0.61) 7519.19: diff (fsmonitor=rs-git-fsmonitor) 0.11(0.07+0.04) 7519.20: diff -- 0_files (fsmonitor=rs-git-fsmonitor) 0.11(0.09+0.03) 7519.21: diff -- 10_files (fsmonitor=rs-git-fsmonitor) 0.11(0.09+0.03) 7519.22: diff -- 100_files (fsmonitor=rs-git-fsmonitor) 0.11(0.07+0.04) 7519.23: diff -- 1000_files (fsmonitor=rs-git-fsmonitor) 0.11(0.06+0.06) 7519.24: diff -- 10000_files (fsmonitor=rs-git-fsmonitor) 0.11(0.06+0.06) 7519.25: add (fsmonitor=rs-git-fsmonitor) 2.03(1.28+0.69) 7519.28: status (fsmonitor=disabled) 0.77(0.59+0.99) 7519.29: status -uno (fsmonitor=disabled) 0.42(0.33+0.85) 7519.30: status -uall (fsmonitor=disabled) 1.59(1.02+1.34) 7519.31: diff (fsmonitor=disabled) 0.35(0.30+0.81) 7519.32: diff -- 0_files (fsmonitor=disabled) 0.11(0.08+0.04) 7519.33: diff -- 10_files (fsmonitor=disabled) 0.11(0.07+0.04) 7519.34: diff -- 100_files (fsmonitor=disabled) 0.11(0.08+0.03) 7519.35: diff -- 1000_files (fsmonitor=disabled) 0.11(0.10+0.02) 7519.36: diff -- 10000_files (fsmonitor=disabled) 0.12(0.07+0.06) 7519.37: add (fsmonitor=disabled) 2.24(1.48+1.44) Signed-off-by: Nipunn Koorapati <nipunn@dropbox.com>
This benchmark covers the git status time for a heavily dirty directory - benchmarking fsmonitor's refresh When running to compare our perl vs rs-git-fsmonitor - we see that the perl script incurs significant overhead - further motivation to provide a faster implementation within git. 7519.7: status (dirty) (fsmonitor=query-watchman) 10.05(7.78+1.56) 7519.20: status (dirty) (fsmonitor=rs-git-fsmonitor) 6.72(4.37+1.64) 7519.33: status (dirty) (fsmonitor=disabled) 5.62(4.24+2.03) Signed-off-by: Nipunn Koorapati <nipunn@dropbox.com>
|
There is an issue in commit f4cf003: |
nipunn1313
force-pushed
the
nk/fsmonitor-perf-suite
branch
from
f4cf003 to
e5b0eee
Compare
|
/submit |
|
Submitted as pull.772.git.1603740773.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
![gitgitgadget[bot]](https://avatars.githubusercontent.com/in/12836?s=60&v=4){kind=small}
| then | ||
| echo "warning: Setting GIT_PERF_REPEAT_COUNT=1" >&2 | ||
| GIT_PERF_REPEAT_COUNT=1 | ||
| fi |
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
"Nipunn Koorapati via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: Nipunn Koorapati <nipunn@dropbox.com>
>
> Previously - it would silently run the perf suite w/o using
> fsmonitor - fsmonitor errors are not hard failures.
> Now it errors loudly.
>
> GIT_PERF_7519_FSMONITOR="$HOME/rs-git-fsmonitorr"
> ./p7519-fsmonitor.sh -i -v
>
> fatal: cannot run /home/nipunn/rs-git-fsmonitorr:
> No such file or directory
> not ok 2 - setup for fsmonitor
>
> Signed-off-by: Nipunn Koorapati <nipunn@dropbox.com>
> ---
> t/perf/p7519-fsmonitor.sh | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/t/perf/p7519-fsmonitor.sh b/t/perf/p7519-fsmonitor.sh
> index 4030f569cf..88b3717e2a 100755
> --- a/t/perf/p7519-fsmonitor.sh
> +++ b/t/perf/p7519-fsmonitor.sh
> @@ -126,7 +126,9 @@ test_expect_success "setup for fsmonitor" '
> fi &&
>
> git config core.fsmonitor "$INTEGRATION_SCRIPT" &&
> - git update-index --fsmonitor &&
> + git update-index --fsmonitor 2>error &&
> + cat error &&
> + [ ! -s error ] && # ensure no silent error
I usually do not review or write t/perf/, but is test_must_be_empty
available to you at this point?
> git status # Warm caches
> '
There was a problem hiding this comment.
On the Git mailing list, Nipunn Koorapati wrote (reply to this):
> I usually do not review or write t/perf/, but is test_must_be_empty
> available to you at this point?
Everything in test-lib should be available - so yes! I can try it out
and switch to it in the next roll of this patch series. I did not
realize this helper was available. Thank you.
--Nipunn
|
User |
|
This branch is now known as |
|
This patch series was integrated into seen via git@f35ec62. |
|
This patch series was integrated into seen via git@29fa2ba. |
|
This patch series was integrated into seen via git@f7e8f03. |
|
This patch series was integrated into seen via git@2506e0c. |
|
This patch series was integrated into seen via git@f7b063a. |
|
This patch series was integrated into seen via git@ec9322f. |
|
This patch series was integrated into seen via git@6dd7057. |
|
This patch series was integrated into seen via git@293d028. |
|
This patch series was integrated into seen via git@5730311. |
|
This patch series was integrated into next via git@9fed160. |
|
This patch series was integrated into seen via git@835f0b7. |
|
This patch series was integrated into seen via git@203ffb5. |
|
This patch series was integrated into seen via git@a643735. |
|
This patch series was integrated into next via git@a643735. |
|
This patch series was integrated into master via git@a643735. |
|
Closed via a643735. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch series builds upon nk/diff-files-vs-fsmonitor
This builds up to a comparison between our perl script and https://github.com/jgavris/rs-git-fsmonitor. Stats on the comparison are in the final commit message. I've found that
rs-git-fsmonitor saves 20-30ms off of every git command compared to the perl script.
It may provide some motivation for supplying a faster implementation of fsmonitor-watchman.
cc: Nipunn Koorapati nipunn1313@gmail.com