Re: [PATCH] t/perf/perf-lib.sh: remove test_times.* at the end test_perf_()
Jeff King <peff@peff.net> writes:
Show 31 quoted lines
> What I'd propose instead is that we ought to have:
>
> test_perf 'checkout'
> --prepare '
> git reset --hard the_original_state
> ' '
> git checkout
> '
>
> Having two multi-line snippets is a bit ugly (check out that awful
> middle line), but I think this could be added without breaking existing
> tests (they just wouldn't have a --prepare option).
>
> If that syntax is too horrendous, we could have:
>
> # this saves the snippet in a variable internally, and runs
> # it before each trial of the next test_perf(), after which
> # it is discarded
> test_perf_prepare '
> git reset --hard the_original_state
> '
>
> test_perf 'checkout' '
> git checkout
> '
>
> I think that would be pretty easy to implement, and would solve the most
> common form of this problem. And there's plenty of prior art; just about
> every decent benchmarking system has a "do this before each trial"
> mechanism. Our t/perf suite (as you probably noticed) is rather more
> ad-hoc and less mature.
Show 6 quoted lines
> There are cases it doesn't help, though. For instance, in one of the
> scripts we measure the time to run "git repack -adb" to generate
> bitmaps. But the first run has to do more work, because we can reuse
> results for subsequent ones! It would help to "rm -f
> objects/pack/*.bitmap", but even that's not entirely fair, as it will be
> repacking from a single pack, versus whatever state we started with.
You need a "do this too for each iteration but do not time it", i.e.
test_perf 'repack performance' --prepare '
make a messy original repository
' --per-iteration-prepare '
prepare a test repository from the messy original
' --time-this-part-only '
git repack -adb
'Syntactically, eh, Yuck.
Junio C Hamano <gitster@pobox.com>