Re: [PATCH v1 0/2] perf/aggregate: sort result by regression

[Christian Couder]

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

[Junio C Hamano]

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

[Christian Couder]

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.