Re: [WIP/PATCH 7/6] perf: add a performance test for core.fsmonitor

From: Jeff King <peff@peff.net>
Date: Jun 4, 2017, 08:21 UTC
Message-ID: <20170604082141.54wanlacy5ksaalt@sigill.intra.peff.net>
In-Reply-To: <CACBZZX4XOaN8o89vetoU8NMqRH+BaqHGkxq77MpqzvAM40exEA@mail.gmail.com>

On Sun, Jun 04, 2017 at 09:46:19AM +0200, Ævar Arnfjörð Bjarmason wrote:

Show 12 quoted lines

> What I'm referring to is not a limitation of git (we'll always be able
> to turn off core.fsmonitor), but a limitation of the perf framework.
> There's no way to run a test like this:
> 
>     ./run master next -- p*.sh
> 
> And have some information passed to the test to apply different
> runtime options to the test depending on master or next, or be to test
> master twice, once with the fsmonitor, once without, which this
> hypothetical feature would do:
> 
>     ./run master:"GIT_PERF_7519_NO_FSMONITOR=Y" master -- p*.sh

Yeah, the perf framework was originally designed to find regressions between versions of Git. It's really bad at comparing results between any other dimensions. You can test different repository sizes by tweaking the environment, but it can't aggregate or compare results between those runs. Likewise, you can have two tests in a script which time Git with and without certain options set, but there's no way to compare the results of those tests.

It would be nice if the perf framework was aware of all of these dimensions, stored each result as a tuple of all of the properties (rather than just the version), and then let you group the results along any dimension (I suspect there are cases where multi-dimensional summaries would be useful, but that could come later).

For some dimensions you'd probably want support in the perf scripts themselves to run a test with two variants. E.g., something like:

  test_perf_group 'fsmonitor' \
	'false' 'git config core.fsmonitor false' \
	 'true' 'git config core.fsmonitor true'
  test_perf 'status' 'git status'
  test_perf_group_end

where that would run the "git status" test for each of the named setups, and store the timing results under "($VERSION, fsmonitor=false)", and so on.

That's less flexible than specifying it to "./run" (which would let you run just one tuple if you chose). But it also relieves the burden from the user of figuring out which dimensions are interesting to tweak.

-Peff