v2, 9 partsTeach 'run' perf script to read config files

(It looks like smtp.gmail.com isn't working anymore for me, so I am trying to send this using Gmail for the cover letter and Submitgit for the patches.)

Goal ~~~~

Using many long environment variables to give parameters to the 'run' script is error prone and tiring.

We want to make it possible to store the parameters to the 'run' script in a config file. This makes it easier to store, reuse, share and compare parameters.

It also makes it easy to run series of tests.

Design ~~~~~~

We use the same config format as the ".git/config" file as Git users and developers are familiar with this nice format and have great tools to change, query and manage it.

We use values from the config file to set the environment variables that are used by the scripts if they are not already set.

We want to make it possible to run series of tests by passing only a config file to the 'run' script.

For example a config file like the following can be used to run perf tests with Git compiled both with and without libpcre:

[perf]
        dirsOrRevs = v2.12.0 v2.13.0
        repeatCount = 10
[perf "with libpcre"]
        makeOpts = "DEVELOPER=1 USE_LIBPCRE=YesPlease"
[perf "without libpcre"]
        makeOpts = "DEVELOPER=1"

This makes it easy to see what changes between the different runs.

It's also possible (though maybe not so useful) to just separate tests from different versions like this:

[perf]
        repeatCount = 2
        makeOpts = "DEVELOPER=1 USE_LIBPCRE=YesPlease"

[perf "with v2.12.0 and v2.13.1"]
        dirsOrRevs = v2.12.0 v2.13.1
[perf "with v2.11.0 and v2.12.1"]
        dirsOrRevs = v2.11.0 v2.12.1

Highlevel view of the patches in the series ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

  - Patches 1/9 to 4/9 were already in v1 and haven't changed.

Patch 1/9 teaches the '--config <configfile>' option to the 'run' script, but <configfile> is just put into the GIT_PERF_CONFIG_FILE variable which is not used yet.

Patch 2/9 add the get_var_from_env_or_config() function to read config options from the <configfile> and set values to some variables from these config options or from default values.

Patch 3/9 and 4/9 use the get_var_from_env_or_config() function to make it possible to set parameters used by the 'run' script.

  - Patches 5/9 to 9/9 are new.

Patch 5/9 introduce a function to deal with subsections in the config file.

Patch 6/9 improves the get_var_from_env_or_config() function so that it can handle subsections.

Patch 7/9 adds the run_subsection() function to run the tests for a subsection.

Patch 8/9 improves the output when building a rev.

Patch 9/9 stores subsection results into subdirectories of test-results so that results from previous subsections is not overwritten.

Future work ~~~~~~~~~~~

In the future I may work on the following:

  - improving aggregate.perl so that it can aggregates the results in
    different ways and formats, especially so that the results can be
    used by Codespeed (https://github.com/tobami/codespeed)
  - making it possible to configure more things in the config file
  - improving how GIT-BUILD-OPTIONS is handled

Though I think the series does not need the above improvements to be already valuable.

Links ~~~~~

This patch series is also available here:

  https://github.com/chriscool/git/commits/perf-conf

Links to the previous version of this series are:

v1:
  https://github.com/chriscool/git/commits/perf-conf5
  https://public-inbox.org/git/20170713065050.19215-1-chriscool@tuxfamily.org/

Christian Couder (9):
  perf/run: add '--config' option to the 'run' script
  perf/run: add get_var_from_env_or_config()
  perf/run: add GIT_PERF_DIRS_OR_REVS
  perf/run: add calls to get_var_from_env_or_config()
  perf/run: add get_subsections()
  perf/run: update get_var_from_env_or_config() for subsections
  perf/run: add run_subsection()
  perf/run: show name of rev being built
  perf: store subsection results in "test-results/$GIT_PERF_SUBSECTION/"

 t/perf/aggregate.perl | 11 +++++--
 t/perf/perf-lib.sh    |  4 +--
 t/perf/run            | 89 +++++++++++++++++++++++++++++++++++++++++++++------
 3 files changed, 89 insertions(+), 15 deletions(-)

Christian Couder <christian.couder@gmail.com> writes:

SubmitGit may want to learn the "change the timestamps of the individual patches by 1 second" trick from "git send-email" to help threading (you can view inbox/comp.version-control.git/ group over nntp and tell your newsreader to sort-by-date).

It isn't quite clear what _did_ change in the series and what lessons were learned form the previous round's discussion here. The sample configuration in the description above (snipped) seems to have been extended and it shows that one of the use cases of the feature is to allow comparing runs against two versions, which looked more or less sensible way to express it.

Thanks.  Let me see how well it works ;-)

On Sun, Sep 24, 2017 at 9:59 AM, Junio C Hamano <gitster@pobox.com> wrote:

Roberto is now in CC. I will let him answer about that.

In the previous round of discussion, I think the reviewers (you and Peff) basically agreed that it was worth improving the test framework to make it possible to easily run many tests, though reviewers were not sure if what I had planned to do would in the end be a good solution.

(See https://public-inbox.org/git/20170713065050.19215-1-chriscool@tuxfamily.org/)

So what did change is that I implemented what was in the "Future work" section in v1.

[...]

Thanks for testing ;-)

On 26 September 2017 at 16:40, Christian Couder <christian.couder@gmail.com> wrote:

I had a quick look at git-send-email.perl, I see the trick is the `time++` one introduced with https://github.com/git/git/commit/a5370b16 - seems reasonable!

SubmitGit makes all emails in-reply-to the initial email, which I think is correct behaviour, but I can see that offsetting the times would probably give a more reliable sorting in a newsreader. Unfortunately the documentation for AWS Simple Email Service (SES) says:

  "Note: Amazon SES overrides any Date header you provide with the
time that Amazon
  SES accepts the message."

http://docs.aws.amazon.com/ses/latest/DeveloperGuide/header-fields.html

...so the only way SubmitGit can offset the times is to literally delay the sending of the emails, which is a bit unfortunate for patchbombs more than a few dozen commits long!

I'll take a further look at this when I get a bit more free time.

Roberto

How many series of more than "a few dozen" patches were sent to the list lately? I'd argue this is a non issue for the typical use case of submitGit.

Thanks! Stefan

Roberto Tyley <roberto.tyley@gmail.com> writes:

As this matters mostly for a series that is longer than several patches, it indeed is unfortunate. If SubmitGit needs to send and sleep for a dozen messages, does it mean the end user has to wait 12 (or is it 11? ;-)) seconds? If the human is the only thing that needs waiting, it may be OK (after all, it all happens in the web browser and the human can switch to other tasks after clicking "submit"), but that may not be acceptable if this artificial delay can cause a timeout in a moving part among many.

Thanks for looking into this.