Re: [PATCH 2/4] fsmonitor: Don't bother pretty-printing JSON from watchman

[Johannes Schindelin]

Hi Alex,

On Wed, 25 Oct 2017, Alex Vandiver wrote:

> [Johannes asked about the speedup when using --no-pretty]
>
> ...so a modest 8% speedup.  I note that those numbers are for a perl
> with JSON::XS installed; without it installed, the runtime is so long
> that I gave up waiting for it.

Very nice! AFAIR Git for Windows does *not* come with JSON::XS.

> Anyways, I'll put that in the commit message in the re-roll.

Thank you,
Johannes

Re: [PATCH 2/4] fsmonitor: Don't bother pretty-printing JSON from watchman

[Alex Vandiver]

On Fri, 20 Oct 2017, Ben Peart wrote:
> > While I am very much infavor of this change (I was not aware of the
> > --no-pretty option), I would like to see some statistics on that. Could
> > you measure the impact, please, and include the results in the commit
> > message?
> > 
> > Ciao,
> > Johannes
> > 
> 
> I was also unaware of the --no-pretty option. I've tested this on Windows
> running version 4.9.0 of Watchman and verified that it does work correctly.
> I'm also curious if it produces any measurable difference in performance.

On a repository with ~160k files, the following test harness, which
requests all files inside the repository and parses that output:

--8<---
#!/usr/bin/perl

use strict;
use warnings;
use IPC::Open2;

my $pid = open2(\*CHLD_OUT, \*CHLD_IN, "watchman -j @ARGV")
or die "open2() failed: $!\n" .
"Falling back to scanning...\n";

my $query = qq|["query", "$ENV{PWD}", {}]|;

print CHLD_IN $query;
close CHLD_IN;
my $response = do {local $/; };

my $json_pkg;
eval {
require JSON::XS;
$json_pkg = "JSON::XS";
1;
} or do {
require JSON::PP;
$json_pkg = "JSON::PP";
};

my $o = $json_pkg->new->utf8->decode($response);
--8<---

...run with dumbbench[1], produces:

$ dumbbench -- ./test.pl
cmd: Ran 22 iterations (2 outliers).
cmd: Rounded run time per iteration: 5.240e+00 +/- 1.1e-02 (0.2%)
$ dumbbench -- ./test.pl --no-pretty
cmd: Ran 21 iterations (1 outliers).
cmd: Rounded run time per iteration: 4.866e+00 +/- 1.3e-02 (0.3%)

...so a modest 8% speedup.  I note that those numbers are for a perl
with JSON::XS installed; without it installed, the runtime is so long
that I gave up waiting for it.

Anyways, I'll put that in the commit message in the re-roll.
 - Alex


[1] https://metacpan.org/release/Dumbbench

Re: [PATCH 2/4] fsmonitor: Don't bother pretty-printing JSON from watchman

[Ben Peart]

On 10/20/2017 9:00 AM, Johannes Schindelin wrote:

Hi Alex,

On Thu, 19 Oct 2017, Alex Vandiver wrote:


This provides small performance savings.

diff --git a/t/t7519/fsmonitor-watchman b/t/t7519/fsmonitor-watchman
index 377edc7be..eba46c78b 100755
--- a/t/t7519/fsmonitor-watchman
+++ b/t/t7519/fsmonitor-watchman
@@ -51,7 +51,7 @@ launch_watchman();
  
  sub launch_watchman {
  
-	my $pid = open2(\*CHLD_OUT, \*CHLD_IN, 'watchman -j')

+   my $pid = open2(\*CHLD_OUT, \*CHLD_IN, 'watchman -j --no-pretty')


While I am very much infavor of this change (I was not aware of the
--no-pretty option), I would like to see some statistics on that. Could
you measure the impact, please, and include the results in the commit
message?

Ciao,
Johannes



I was also unaware of the --no-pretty option. I've tested this on 
Windows running version 4.9.0 of Watchman and verified that it does work 
correctly.  I'm also curious if it produces any measurable difference in 
performance.

Re: [PATCH 2/4] fsmonitor: Don't bother pretty-printing JSON from watchman

[Johannes Schindelin]

Hi Alex,

On Thu, 19 Oct 2017, Alex Vandiver wrote:

> This provides small performance savings.
> 
> diff --git a/t/t7519/fsmonitor-watchman b/t/t7519/fsmonitor-watchman
> index 377edc7be..eba46c78b 100755
> --- a/t/t7519/fsmonitor-watchman
> +++ b/t/t7519/fsmonitor-watchman
> @@ -51,7 +51,7 @@ launch_watchman();
>  
>  sub launch_watchman {
>  
> - my $pid = open2(\*CHLD_OUT, \*CHLD_IN, 'watchman -j')
> + my $pid = open2(\*CHLD_OUT, \*CHLD_IN, 'watchman -j --no-pretty')

While I am very much infavor of this change (I was not aware of the
--no-pretty option), I would like to see some statistics on that. Could
you measure the impact, please, and include the results in the commit
message?

Ciao,
Johannes

[PATCH 2/4] fsmonitor: Don't bother pretty-printing JSON from watchman

[Alex Vandiver]

This provides small performance savings.

Signed-off-by: Alex Vandiver 
---
 t/t7519/fsmonitor-watchman | 2 +-
 templates/hooks--fsmonitor-watchman.sample | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t7519/fsmonitor-watchman b/t/t7519/fsmonitor-watchman
index 377edc7be..eba46c78b 100755
--- a/t/t7519/fsmonitor-watchman
+++ b/t/t7519/fsmonitor-watchman
@@ -51,7 +51,7 @@ launch_watchman();
 
 sub launch_watchman {
 
-   my $pid = open2(\*CHLD_OUT, \*CHLD_IN, 'watchman -j')
+   my $pid = open2(\*CHLD_OUT, \*CHLD_IN, 'watchman -j --no-pretty')
or die "open2() failed: $!\n" .
"Falling back to scanning...\n";
 
diff --git a/templates/hooks--fsmonitor-watchman.sample 
b/templates/hooks--fsmonitor-watchman.sample
index 7df590d29..60eb7e70b 100755
--- a/templates/hooks--fsmonitor-watchman.sample
+++ b/templates/hooks--fsmonitor-watchman.sample
@@ -50,7 +50,7 @@ launch_watchman();
 
 sub launch_watchman {
 
-   my $pid = open2(\*CHLD_OUT, \*CHLD_IN, 'watchman -j')
+   my $pid = open2(\*CHLD_OUT, \*CHLD_IN, 'watchman -j --no-pretty')
or die "open2() failed: $!\n" .
"Falling back to scanning...\n";
 
-- 
2.15.0.rc1.417.g05670bb6e