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

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 23, 2018, 21:16 UTC
Message-ID: <xmqq7eq2ijc6.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20180323140007.12096-1-chriscool@tuxfamily.org>

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

Show 6 quoted lines

> 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".