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

From: Alex Vandiver <alexmv@dropbox.com>
Date: Oct 26, 2017, 00:44 UTC
Message-ID: <alpine.DEB.2.10.1710251722310.9817@alexmv-linux>
In-Reply-To: <c43315f3-4155-6b07-bde2-5855dceacf80@gmail.com>

On Fri, 20 Oct 2017, Ben Peart wrote:

Show 12 quoted lines

> > 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 $/; <CHLD_OUT>};

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