Re: [PATCH v1 0/2] perf/aggregate: sort result by regression
On Fri, Mar 23, 2018 at 10:16 PM, Junio C Hamano wrote: > Christian Couder writes: > >> This small patch series makes it easy to spot big performance >> regressions, so that they can later be investigated. >> >> For example: >> >> $ ./aggregate.perl --sortbyregression --subsection "without libpcre" v2.14.3 >> v2.15.1 v2.16.2 p4220-log-grep-engines.sh > > Are we comfortable with the idea that other kinds of sorting, when > invented in the future, would have to say > > $ ./aggregate.perl --sortbysomethingelse --subsection "without libpcre" \ > A B C p4220-log-grep-engines.sh > > or will we regret that and wish if we could write it as > > $ ./aggregate.perl --sort-by=somethingelse --subsection "without libpcre" > \ > A B C p4220-log-grep-engines.sh > > If the latter, perhaps we should use --soft-by=regression from day one. Yeah, I think it is probably better to use --sort-by=regression (not --soft-by ;-), so I will use that in the next version. > Do we expect that "taking a lot more more rtime than the previous" > will stay to be the only kind of "regression" we care about in the > context of t/perf? If so, there is no need for further suggestion, > but if not, perhaps we should plan if/how we could also parameterize > the "rtime" part from the command line. E.g. > > $ ./aggregate.perl --sort-by=regression:stime > > might be a way to say "we only care about the stime part" in the > future, even though --sort-by=regression may be a short-hand to say > "we care about rtime regression" i.e. "--sort-by=regression:rtime". Yeah, I think we can have the short form "--sort-by=regression" mean "--sort-by=regression:rtime" and skip implementing the long form. I will talk about the long form in the commit message.
Re: [PATCH v1 0/2] perf/aggregate: sort result by regression
Christian Couder writes: > This small patch series makes it easy to spot big performance > regressions, so that they can later be investigated. > > For example: > > $ ./aggregate.perl --sortbyregression --subsection "without libpcre" v2.14.3 > v2.15.1 v2.16.2 p4220-log-grep-engines.sh Are we comfortable with the idea that other kinds of sorting, when invented in the future, would have to say $ ./aggregate.perl --sortbysomethingelse --subsection "without libpcre" \ A B C p4220-log-grep-engines.sh or will we regret that and wish if we could write it as $ ./aggregate.perl --sort-by=somethingelse --subsection "without libpcre" \ A B C p4220-log-grep-engines.sh If the latter, perhaps we should use --soft-by=regression from day one. Do we expect that "taking a lot more more rtime than the previous" will stay to be the only kind of "regression" we care about in the context of t/perf? If so, there is no need for further suggestion, but if not, perhaps we should plan if/how we could also parameterize the "rtime" part from the command line. E.g. $ ./aggregate.perl --sort-by=regression:stime might be a way to say "we only care about the stime part" in the future, even though --sort-by=regression may be a short-hand to say "we care about rtime regression" i.e. "--sort-by=regression:rtime".
Re: [PATCH v1 0/2] perf/aggregate: sort result by regression
On Fri, Mar 23, 2018 at 3:00 PM, Christian Couder wrote: > This small patch series makes it easy to spot big performance > regressions, so that they can later be investigated. Sorry I just realized there are indent problems in the patches.
[PATCH v1 0/2] perf/aggregate: sort result by regression
This small patch series makes it easy to spot big performance regressions, so that they can later be investigated. For example: $ ./aggregate.perl --sortbyregression --subsection "without libpcre" v2.14.3 v2.15.1 v2.16.2 p4220-log-grep-engines.sh +5.0% p4220-log-grep-engines.2 0.60(0.58+0.02) 0.63(0.59+0.04) v2.14.3 v2.15.1 +4.5% p4220-log-grep-engines.10 0.67(0.64+0.03) 0.70(0.67+0.02) v2.14.3 v2.15.1 +1.7% p4220-log-grep-engines.5 0.58(0.57+0.01) 0.59(0.59+0.00) v2.14.3 v2.15.1 +1.7% p4220-log-grep-engines.6 0.58(0.58+0.00) 0.59(0.56+0.02) v2.14.3 v2.15.1 +1.7% p4220-log-grep-engines.17 0.58(0.57+0.01) 0.59(0.56+0.02) v2.14.3 v2.15.1 +1.7% p4220-log-grep-engines.1 0.60(0.58+0.01) 0.61(0.60+0.01) v2.14.3 v2.15.1 +1.6% p4220-log-grep-engines.13 0.64(0.63+0.02) 0.65(0.63+0.01) v2.14.3 v2.15.1 +1.5% p4220-log-grep-engines.9 0.67(0.66+0.01) 0.68(0.67+0.01) v2.14.3 v2.15.1 +0.0% p4220-log-grep-engines.14 0.65(0.62+0.02) 0.65(0.63+0.02) v2.14.3 v2.15.1 +0.0% p4220-log-grep-engines.18 0.58(0.57+0.00) 0.58(0.56+0.02) v2.14.3 v2.15.1 -1.5% p4220-log-grep-engines.13 0.65(0.63+0.01) 0.64(0.62+0.01) v2.15.1 v2.16.2 -1.5% p4220-log-grep-engines.14 0.65(0.63+0.02) 0.64(0.60+0.03) v2.15.1 v2.16.2 -1.6% p4220-log-grep-engines.1 0.61(0.60+0.01) 0.60(0.58+0.02) v2.15.1 v2.16.2 -1.7% p4220-log-grep-engines.5 0.59(0.59+0.00) 0.58(0.55+0.02) v2.15.1 v2.16.2 -1.7% p4220-log-grep-engines.6 0.59(0.56+0.02) 0.58(0.55+0.02) v2.15.1 v2.16.2 -1.7% p4220-log-grep-engines.18 0.58(0.56+0.02) 0.57(0.55+0.02) v2.15.1 v2.16.2 -2.9% p4220-log-grep-engines.9 0.68(0.67+0.01) 0.66(0.64+0.02) v2.15.1 v2.16.2 -3.4% p4220-log-grep-engines.17 0.59(0.56+0.02) 0.57(0.55+0.01) v2.15.1 v2.16.2 -4.3% p4220-log-grep-engines.10 0.70(0.67+0.02) 0.67(0.66+0.01) v2.15.1 v2.16.2 -4.8% p4220-log-grep-engines.2 0.63(0.59+0.04) 0.60(0.57+0.03) v2.15.1 v2.16.2 Christian Couder (2): perf/aggregate: add display_dir() perf/aggregate: add --sortbyregression option t/perf/aggregate.perl | 59 +++ 1 file changed, 54 insertions(+), 5 deletions(-) -- 2.17.0.rc0.37.g8f476fabe9
