{"thread":{"id":"46818","subject":"[PATCH v2 0/9] Teach 'run' perf script to read config files","startedAt":"2017-09-23T19:39:08Z","lastAt":"2017-09-27T00:04:26Z","messageCount":6,"participants":["Christian Couder","Junio C Hamano","Roberto Tyley","Stefan Beller"],"isPatch":true,"patchVersion":2,"patchTotal":9},"messages":[{"id":"328738","messageId":"CAP8UFD2j-UFh+9awz91gtZ-jusq7EUOExMgURO59vpf29jXS4A@mail.gmail.com","threadId":"46818","inReplyTo":null,"subject":"[PATCH v2 0/9] Teach 'run' perf script to read config files","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-09-23T19:39:02Z","receivedAt":"2017-09-23T19:39:08Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"(It looks like smtp.gmail.com isn't working anymore for me, so I am\ntrying to send this using Gmail for the cover letter and Submitgit for\nthe patches.)\n\nGoal\n~~~~\n\nUsing many long environment variables to give parameters to the 'run'\nscript is error prone and tiring.\n\nWe want to make it possible to store the parameters to the 'run'\nscript in a config file. This makes it easier to store, reuse,\nshare and compare parameters.\n\nIt also makes it easy to run series of tests.\n\nDesign\n~~~~~~\n\nWe use the same config format as the \".git/config\" file as Git users\nand developers are familiar with this nice format and have great tools\nto change, query and manage it.\n\nWe use values from the config file to set the environment variables\nthat are used by the scripts if they are not already set.\n\nWe want to make it possible to run series of tests by passing only a\nconfig file to the 'run' script.\n\nFor example a config file like the following can be used to run perf\ntests with Git compiled both with and without libpcre:\n\n[perf]\n dirsOrRevs = v2.12.0 v2.13.0\n repeatCount = 10\n[perf \"with libpcre\"]\n makeOpts = \"DEVELOPER=1 USE_LIBPCRE=YesPlease\"\n[perf \"without libpcre\"]\n makeOpts = \"DEVELOPER=1\"\n\nThis makes it easy to see what changes between the different runs.\n\nIt's also possible (though maybe not so useful) to just separate tests\nfrom different versions like this:\n\n[perf]\n repeatCount = 2\n makeOpts = \"DEVELOPER=1 USE_LIBPCRE=YesPlease\"\n\n[perf \"with v2.12.0 and v2.13.1\"]\n dirsOrRevs = v2.12.0 v2.13.1\n[perf \"with v2.11.0 and v2.12.1\"]\n dirsOrRevs = v2.11.0 v2.12.1\n\nHighlevel view of the patches in the series\n~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n\n - Patches 1/9 to 4/9 were already in v1 and haven't changed.\n\nPatch 1/9 teaches the '--config ' option to the 'run'\nscript, but is just put into the GIT_PERF_CONFIG_FILE\nvariable which is not used yet.\n\nPatch 2/9 add the get_var_from_env_or_config() function to read config\noptions from the and set values to some variables from\nthese config options or from default values.\n\nPatch 3/9 and 4/9 use the get_var_from_env_or_config() function to\nmake it possible to set parameters used by the 'run' script.\n\n - Patches 5/9 to 9/9 are new.\n\nPatch 5/9 introduce a function to deal with subsections in the config\nfile.\n\nPatch 6/9 improves the get_var_from_env_or_config() function so that\nit can handle subsections.\n\nPatch 7/9 adds the run_subsection() function to run the tests for a\nsubsection.\n\nPatch 8/9 improves the output when building a rev.\n\nPatch 9/9 stores subsection results into subdirectories of test-results\nso that results from previous subsections is not overwritten.\n\nFuture work\n~~~~~~~~~~~\n\nIn the future I may work on the following:\n\n - improving aggregate.perl so that it can aggregates the results in\n different ways and formats, especially so that the results can be\n used by Codespeed (https://github.com/tobami/codespeed)\n - making it possible to configure more things in the config file\n - improving how GIT-BUILD-OPTIONS is handled\n\nThough I think the series does not need the above improvements to be\nalready valuable.\n\nLinks\n~~~~~\n\nThis patch series is also available here:\n\n https://github.com/chriscool/git/commits/perf-conf\n\nLinks to the previous version of this series are:\n\nv1:\n https://github.com/chriscool/git/commits/perf-conf5\n https://public-inbox.org/git/20170713065050.19215-1-chriscool@tuxfamily.org/\n\nChristian Couder (9):\n perf/run: add '--config' option to the 'run' script\n perf/run: add get_var_from_env_or_config()\n perf/run: add GIT_PERF_DIRS_OR_REVS\n perf/run: add calls to get_var_from_env_or_config()\n perf/run: add get_subsections()\n perf/run: update get_var_from_env_or_config() for subsections\n perf/run: add run_subsection()\n perf/run: show name of rev being built\n perf: store subsection results in \"test-results/$GIT_PERF_SUBSECTION/\"\n\n t/perf/aggregate.perl | 11 +++++--\n t/perf/perf-lib.sh | 4 +--\n t/perf/run | 89 +++++++++++++++++++++++++++++++++++++++++++++------\n 3 files changed, 89 insertions(+), 15 deletions(-)\n\n-- \n2.14.1.767.g2dbbf9317b\n"},{"id":"328763","messageId":"xmqqbmm0h6h1.fsf@gitster.mtv.corp.google.com","threadId":"46818","inReplyTo":"CAP8UFD2j-UFh+9awz91gtZ-jusq7EUOExMgURO59vpf29jXS4A@mail.gmail.com","subject":"Re: [PATCH v2 0/9] Teach 'run' perf script to read config files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-09-24T07:59:54Z","receivedAt":"2017-09-24T08:00:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> (It looks like smtp.gmail.com isn't working anymore for me, so I am\n> trying to send this using Gmail for the cover letter and Submitgit for\n> the patches.)\n\nSubmitGit may want to learn the \"change the timestamps of the\nindividual patches by 1 second\" trick from \"git send-email\" to help\nthreading (you can view inbox/comp.version-control.git/ group over\nnntp and tell your newsreader to sort-by-date).\n\n> Highlevel view of the patches in the series\n> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n>\n> - Patches 1/9 to 4/9 were already in v1 and haven't changed.\n\nIt isn't quite clear what _did_ change in the series and what\nlessons were learned form the previous round's discussion here. The\nsample configuration in the description above (snipped) seems to\nhave been extended and it shows that one of the use cases of the\nfeature is to allow comparing runs against two versions, which\nlooked more or less sensible way to express it.\n\n> Christian Couder (9):\n> perf/run: add '--config' option to the 'run' script\n> perf/run: add get_var_from_env_or_config()\n> perf/run: add GIT_PERF_DIRS_OR_REVS\n> perf/run: add calls to get_var_from_env_or_config()\n> perf/run: add get_subsections()\n> perf/run: update get_var_from_env_or_config() for subsections\n> perf/run: add run_subsection()\n> perf/run: show name of rev being built\n> perf: store subsection results in \"test-results/$GIT_PERF_SUBSECTION/\"\n>\n> t/perf/aggregate.perl | 11 +++++--\n> t/perf/perf-lib.sh | 4 +--\n> t/perf/run | 89 +++++++++++++++++++++++++++++++++++++++++++++------\n> 3 files changed, 89 insertions(+), 15 deletions(-)\n\nThanks. Let me see how well it works ;-)\n"},{"id":"328950","messageId":"CAP8UFD1C80cHnMtdZ-iTYQpNNErUEJ9TmQ9baG1J2w+pv1ceSw@mail.gmail.com","threadId":"46818","inReplyTo":"xmqqbmm0h6h1.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 0/9] Teach 'run' perf script to read config files","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-09-26T15:40:24Z","receivedAt":"2017-09-26T15:40:30Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Sun, Sep 24, 2017 at 9:59 AM, Junio C Hamano wrote:\n> Christian Couder writes:\n>\n>> (It looks like smtp.gmail.com isn't working anymore for me, so I am\n>> trying to send this using Gmail for the cover letter and Submitgit for\n>> the patches.)\n>\n> SubmitGit may want to learn the \"change the timestamps of the\n> individual patches by 1 second\" trick from \"git send-email\" to help\n> threading (you can view inbox/comp.version-control.git/ group over\n> nntp and tell your newsreader to sort-by-date).\n\nRoberto is now in CC. I will let him answer about that.\n\n>> Highlevel view of the patches in the series\n>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n>>\n>> - Patches 1/9 to 4/9 were already in v1 and haven't changed.\n>\n> It isn't quite clear what _did_ change in the series and what\n> lessons were learned form the previous round's discussion here.\n\nIn the previous round of discussion, I think the reviewers (you and\nPeff) basically agreed that it was worth improving the test framework\nto make it possible to easily run many tests, though reviewers were\nnot sure if what I had planned to do would in the end be a good\nsolution.\n\n(See https://public-inbox.org/git/20170713065050.19215-1-chriscool@tuxfamily.org/)\n\nSo what did change is that I implemented what was in the \"Future work\"\nsection in v1.\n\n> The\n> sample configuration in the description above (snipped) seems to\n> have been extended and it shows that one of the use cases of the\n> feature is to allow comparing runs against two versions, which\n> looked more or less sensible way to express it.\n\n[...]\n\n> Thanks. Let me see how well it works ;-)\n\nThanks for testing ;-)\n"},{"id":"328959","messageId":"CAFY1edZ6FX6s+H_XWa-=nKqr4NA9BNVxR6fcOo+5gn-Z1XKdUg@mail.gmail.com","threadId":"46818","inReplyTo":"CAP8UFD1C80cHnMtdZ-iTYQpNNErUEJ9TmQ9baG1J2w+pv1ceSw@mail.gmail.com","subject":"Re: [PATCH v2 0/9] Teach 'run' perf script to read config files","fromName":"Roberto Tyley","fromEmail":"roberto.tyley@gmail.com","sentAt":"2017-09-26T19:33:16Z","receivedAt":"2017-09-26T19:33:23Z","isPatch":true,"sender":{"key":"roberto.tyley@gmail.com","avatar":"https://avatars.githubusercontent.com/u/52038?v=4"},"body":"On 26 September 2017 at 16:40, Christian Couder\n wrote:\n> On Sun, Sep 24, 2017 at 9:59 AM, Junio C Hamano wrote:\n>> Christian Couder writes:\n>>\n>>> (It looks like smtp.gmail.com isn't working anymore for me, so I am\n>>> trying to send this using Gmail for the cover letter and Submitgit for\n>>> the patches.)\n>>\n>> SubmitGit may want to learn the \"change the timestamps of the\n>> individual patches by 1 second\" trick from \"git send-email\" to help\n>> threading (you can view inbox/comp.version-control.git/ group over\n>> nntp and tell your newsreader to sort-by-date).\n>\n> Roberto is now in CC. I will let him answer about that.\n\nI had a quick look at git-send-email.perl, I see the trick is the `time++` one\nintroduced with https://github.com/git/git/commit/a5370b16 - seems reasonable!\n\nSubmitGit makes all emails in-reply-to the initial email, which I\nthink is correct behaviour,\nbut I can see that offsetting the times would probably give a more\nreliable sorting in\na newsreader. Unfortunately the documentation for AWS Simple Email Service (SES)\nsays:\n\n \"Note: Amazon SES overrides any Date header you provide with the\ntime that Amazon\n SES accepts the message.\"\n\nhttp://docs.aws.amazon.com/ses/latest/DeveloperGuide/header-fields.html\n\n...so the only way SubmitGit can offset the times is to literally\ndelay the sending of the emails,\nwhich is a bit unfortunate for patchbombs more than a few dozen commits long!\n\nI'll take a further look at this when I get a bit more free time.\n\nRoberto\n"},{"id":"328960","messageId":"CAGZ79kZzgBYLRHWcVZX9BcWBvrg4gQa0Y1f+A357hi-L4j+v2Q@mail.gmail.com","threadId":"46818","inReplyTo":"CAFY1edZ6FX6s+H_XWa-=nKqr4NA9BNVxR6fcOo+5gn-Z1XKdUg@mail.gmail.com","subject":"Re: [PATCH v2 0/9] Teach 'run' perf script to read config files","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-09-26T19:38:38Z","receivedAt":"2017-09-26T19:38:44Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"> \"Note: Amazon SES overrides any Date header you provide with the\n> time that Amazon\n> SES accepts the message.\"\n>\n> http://docs.aws.amazon.com/ses/latest/DeveloperGuide/header-fields.html\n>\n> ...so the only way SubmitGit can offset the times is to literally\n> delay the sending of the emails,\n> which is a bit unfortunate for patchbombs more than a few dozen commits long!\n\nHow many series of more than \"a few dozen\" patches were sent to the list lately?\nI'd argue this is a non issue for the typical use case of submitGit.\n\n> I'll take a further look at this when I get a bit more free time.\n\nThanks!\nStefan\n"},{"id":"328986","messageId":"xmqq4lrp9fct.fsf@gitster.mtv.corp.google.com","threadId":"46818","inReplyTo":"CAFY1edZ6FX6s+H_XWa-=nKqr4NA9BNVxR6fcOo+5gn-Z1XKdUg@mail.gmail.com","subject":"Re: [PATCH v2 0/9] Teach 'run' perf script to read config files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-09-27T00:04:18Z","receivedAt":"2017-09-27T00:04:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Roberto Tyley writes:\n\n> I had a quick look at git-send-email.perl, I see the trick is the `time++` one\n> introduced with https://github.com/git/git/commit/a5370b16 - seems reasonable!\n>\n> SubmitGit makes all emails in-reply-to the initial email, which I\n> think is correct behaviour,\n> but I can see that offsetting the times would probably give a more\n> reliable sorting in\n> a newsreader. ...\n> ...so the only way SubmitGit can offset the times is to literally\n> delay the sending of the emails,\n> which is a bit unfortunate for patchbombs more than a few dozen commits long!\n\nAs this matters mostly for a series that is longer than several\npatches, it indeed is unfortunate. If SubmitGit needs to send and\nsleep for a dozen messages, does it mean the end user has to wait 12\n(or is it 11? ;-)) seconds? If the human is the only thing that\nneeds waiting, it may be OK (after all, it all happens in the web\nbrowser and the human can switch to other tasks after clicking\n\"submit\"), but that may not be acceptable if this artificial delay\ncan cause a timeout in a moving part among many.\n\nThanks for looking into this. \n"}]}