diff mbox series

[22/23] p7519: add fsmonitor--daemon

Message ID da5094e52032e28337cfcecd421dedb560952869.1617291666.git.gitgitgadget@gmail.com (mailing list archive)
State New, archived
Headers show
Series Builtin FSMonitor Feature | expand

Commit Message

Jeff Hostetler April 1, 2021, 3:41 p.m. UTC 
From: Jeff Hostetler <jeffhost@microsoft.com>

Repeat all of the fsmonitor perf tests using `git fsmonitor--daemon` and
the "Simple IPC" interface.

Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
---
 t/perf/p7519-fsmonitor.sh | 37 +++++++++++++++++++++++++++++++++++--
 1 file changed, 35 insertions(+), 2 deletions(-)

Comments

Derrick Stolee April 27, 2021, 3:45 p.m. UTC | #1 
On 4/1/2021 11:41 AM, Jeff Hostetler via GitGitGadget wrote:
> From: Jeff Hostetler <jeffhost@microsoft.com>
> 
> Repeat all of the fsmonitor perf tests using `git fsmonitor--daemon` and
> the "Simple IPC" interface.

It would be nice to see some numbers for how this test performs
on some standard Git repositories across Windows and macOS.

> Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
> ---
>  t/perf/p7519-fsmonitor.sh | 37 +++++++++++++++++++++++++++++++++++--
>  1 file changed, 35 insertions(+), 2 deletions(-)
> 
> diff --git a/t/perf/p7519-fsmonitor.sh b/t/perf/p7519-fsmonitor.sh
> index 5eb5044a103c..2d018bc7d589 100755
> --- a/t/perf/p7519-fsmonitor.sh
> +++ b/t/perf/p7519-fsmonitor.sh
> @@ -24,7 +24,8 @@ test_description="Test core.fsmonitor"
>  # GIT_PERF_7519_SPLIT_INDEX: used to configure core.splitIndex
>  # GIT_PERF_7519_FSMONITOR: used to configure core.fsMonitor. May be an
>  #   absolute path to an integration. May be a space delimited list of
> -#   absolute paths to integrations.
> +#   absolute paths to integrations.  (This hook or list of hooks does not
> +#   include the built-in fsmonitor--daemon.)
>  #
>  # The big win for using fsmonitor is the elimination of the need to scan the
>  # working directory looking for changed and untracked files. If the file
> @@ -135,10 +136,16 @@ test_expect_success "one time repo setup" '
>  
>  setup_for_fsmonitor() {
>  	# set INTEGRATION_SCRIPT depending on the environment
> -	if test -n "$INTEGRATION_PATH"
> +	if test -n "$USE_FSMONITOR_DAEMON"
>  	then
> +		git config core.useBuiltinFSMonitor true &&
> +		INTEGRATION_SCRIPT=false
> +	elif test -n "$INTEGRATION_PATH"
> +	then
> +		git config core.useBuiltinFSMonitor false &&
>  		INTEGRATION_SCRIPT="$INTEGRATION_PATH"
>  	else
> +		git config core.useBuiltinFSMonitor false &&
>  		#
>  		# Choose integration script based on existence of Watchman.
>  		# Fall back to an empty integration script.
> @@ -285,4 +292,30 @@ test_expect_success "setup without fsmonitor" '
>  test_fsmonitor_suite
>  trace_stop
>  
> +#
> +# Run a full set of perf tests using the built-in fsmonitor--daemon.
> +# It does not use the Hook API, so it has a different setup.
> +# Explicitly start the daemon here and before we start client commands
> +# so that we can later add custom tracing.
> +#
> +
> +test_lazy_prereq HAVE_FSMONITOR_DAEMON '
> +	git version --build-options | grep "feature:" | grep "fsmonitor--daemon"
> +'

Here you do create the prereq. Let's put this into t/test-lib.sh
or t/test-lib-functions.sh, whichever is more appropriate.

> +
> +if test_have_prereq HAVE_FSMONITOR_DAEMON
> +then
> +	USE_FSMONITOR_DAEMON=t
> +
> +	trace_start fsmonitor--daemon--server
> +	git fsmonitor--daemon --start
> +
> +	trace_start fsmonitor--daemon--client
> +	test_expect_success "setup for fsmonitor--daemon" 'setup_for_fsmonitor'

Maybe this is copied from the rest of the file, but we should probably
use the standard layout for tests here:

	test_expect_success 'setup for fsmonitor--daemon' '
		setup_for_fsmonitor
	'

> +	test_fsmonitor_suite
> +
> +	git fsmonitor--daemon --stop
> +	trace_stop
> +fi
> +

Thanks,
-Stolee
diff mbox series

Patch

diff --git a/t/perf/p7519-fsmonitor.sh b/t/perf/p7519-fsmonitor.sh
index 5eb5044a103c..2d018bc7d589 100755
--- a/t/perf/p7519-fsmonitor.sh
+++ b/t/perf/p7519-fsmonitor.sh
@@ -24,7 +24,8 @@  test_description="Test core.fsmonitor"
 # GIT_PERF_7519_SPLIT_INDEX: used to configure core.splitIndex
 # GIT_PERF_7519_FSMONITOR: used to configure core.fsMonitor. May be an
 #   absolute path to an integration. May be a space delimited list of
-#   absolute paths to integrations.
+#   absolute paths to integrations.  (This hook or list of hooks does not
+#   include the built-in fsmonitor--daemon.)
 #
 # The big win for using fsmonitor is the elimination of the need to scan the
 # working directory looking for changed and untracked files. If the file
@@ -135,10 +136,16 @@  test_expect_success "one time repo setup" '
 
 setup_for_fsmonitor() {
 	# set INTEGRATION_SCRIPT depending on the environment
-	if test -n "$INTEGRATION_PATH"
+	if test -n "$USE_FSMONITOR_DAEMON"
 	then
+		git config core.useBuiltinFSMonitor true &&
+		INTEGRATION_SCRIPT=false
+	elif test -n "$INTEGRATION_PATH"
+	then
+		git config core.useBuiltinFSMonitor false &&
 		INTEGRATION_SCRIPT="$INTEGRATION_PATH"
 	else
+		git config core.useBuiltinFSMonitor false &&
 		#
 		# Choose integration script based on existence of Watchman.
 		# Fall back to an empty integration script.
@@ -285,4 +292,30 @@  test_expect_success "setup without fsmonitor" '
 test_fsmonitor_suite
 trace_stop
 
+#
+# Run a full set of perf tests using the built-in fsmonitor--daemon.
+# It does not use the Hook API, so it has a different setup.
+# Explicitly start the daemon here and before we start client commands
+# so that we can later add custom tracing.
+#
+
+test_lazy_prereq HAVE_FSMONITOR_DAEMON '
+	git version --build-options | grep "feature:" | grep "fsmonitor--daemon"
+'
+
+if test_have_prereq HAVE_FSMONITOR_DAEMON
+then
+	USE_FSMONITOR_DAEMON=t
+
+	trace_start fsmonitor--daemon--server
+	git fsmonitor--daemon --start
+
+	trace_start fsmonitor--daemon--client
+	test_expect_success "setup for fsmonitor--daemon" 'setup_for_fsmonitor'
+	test_fsmonitor_suite
+
+	git fsmonitor--daemon --stop
+	trace_stop
+fi
+
 test_done