All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Jonathan Tan <jonathantanmy@google.com>, git@vger.kernel.org
Subject: Re: [PATCH] t/perf: export variable used in other blocks
Date: Fri, 3 Mar 2017 17:31:27 -0500	[thread overview]
Message-ID: <20170303223127.vqqkmxmcclkmw6wm@sigill.intra.peff.net> (raw)
In-Reply-To: <xmqqbmti9os2.fsf@gitster.mtv.corp.google.com>

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

> 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

  reply	other threads:[~2017-03-03 22:31 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-25  1:18 [PATCH 0/3] Test fetch-pack's ability to fetch arbitrary blobs Jonathan Tan
2017-02-25  1:18 ` [PATCH 1/3] revision: unify {tree,blob}_objects in rev_info Jonathan Tan
2017-02-28 21:42   ` Junio C Hamano
2017-02-28 21:59     ` Jeff King
2017-03-02 18:36       ` Junio C Hamano
2017-02-28 22:06   ` Jeff King
2017-02-25  1:18 ` [PATCH 2/3] revision: exclude trees/blobs given commit Jonathan Tan
2017-02-28 21:44   ` Junio C Hamano
2017-02-28 22:12   ` Jeff King
2017-03-02 19:50     ` [PATCH] t/perf: export variable used in other blocks Jonathan Tan
2017-03-03  6:45       ` Jeff King
2017-03-03  7:14         ` [PATCH] t/perf: use $MODERN_GIT for all repo-copying steps Jeff King
2017-03-03  7:36           ` [PATCH] t/perf: add fallback for pre-bin-wrappers versions of git Jeff King
2017-03-03 18:51         ` [PATCH] t/perf: export variable used in other blocks Junio C Hamano
2017-03-03 22:31           ` Jeff King [this message]
2017-02-28 23:12   ` [PATCH 2/3] revision: exclude trees/blobs given commit Junio C Hamano
2017-02-25  1:18 ` [PATCH 3/3] upload-pack: compute blob reachability correctly Jonathan Tan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170303223127.vqqkmxmcclkmw6wm@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jonathantanmy@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.