Re: [PATCH] t/perf: export variable used in other blocks

From: Jeff King <peff@peff.net>
Date: Mar 3, 2017, 22:31 UTC
Message-ID: <20170303223127.vqqkmxmcclkmw6wm@sigill.intra.peff.net>
In-Reply-To: <xmqqbmti9os2.fsf@gitster.mtv.corp.google.com>

On Fri, Mar 03, 2017 at 10:51:57AM -0800, Junio C Hamano wrote:

Show 14 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> > Thanks, this is obviously the right thing to do, and the mistake is mine
> > from ea97002fc (t/perf: time rev-list with UNINTERESTING commits,
> > 2014-01-20). This is not the first time I've been confused by missing
> > variables in t/perf scripts, since it behaves differently than the
> > normal test suite. I wonder if we should turn on "set -a" during t/perf
> > setup snippets. That's a bit of a blunt tool, but I suspect it would
> > just be easier to work with.
> 
> I wonder if we can make t/perf to behave more similar to the normal
> test suite to eliminate the need for this exporting, by the way.
> t/perf/README does not say anything more than "for lack of better
> options" throughout its history, which does not help very much.

The problem is that it has to run each block multiple times under an external "time" program. If you assume that the shell running the perf suite is bash, you can do:

  i=0
  foo() { echo $i; i=$(($i+1)); }
  for run in 1 2 3; do time foo; done

which shows that you can get repeated timings from a shell function. That shell function would run the actual test snippet, and would have to do some redirect trickery to split the "time" results from the stderr of the underlying test. And parse bash's time output.

But those are all do-able things. The main thing is throwing out a dependency on an external "time" for "bash". That may not be a big loss, though.

-Peff