| Message ID | b3b2574d4d9d10f226b52d81fe0e6bf1f761504e.1611098616.git.me@ttaylorr.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | repack: support repacking into a geometric sequence | expand |
Taylor Blau <me@ttaylorr.com> writes: > From: Jeff King <peff@peff.net> Not a fault of this series at all, but before the precontext of the first hunk, there is > diff --git a/t/perf/p5303-many-packs.sh b/t/perf/p5303-many-packs.sh > index 277d22ec4b..85b077b72b 100755 > --- a/t/perf/p5303-many-packs.sh > +++ b/t/perf/p5303-many-packs.sh > @@ -27,8 +27,11 @@ repack_into_n () { this construct: ... | sed -n '1~5p' | head -n "$1" | ... which is a GNUism. Peff often says that very small population actually run our perf suite, and this seems to corroborate the conjecture. > >pushes && > > # create base packfile > - head -n 1 pushes | > - git pack-objects --delta-base-offset --revs staging/pack && > + base_pack=$( > + head -n 1 pushes | > + git pack-objects --delta-base-offset --revs staging/pack > + ) && > + test_export base_pack && > > # and then incrementals between each pair of commits > last= && > @@ -87,6 +90,15 @@ do > --reflog --indexed-objects --delta-base-offset \ > --stdout </dev/null >/dev/null > ' > + > + test_perf "repack with keep ($nr_packs)" ' > + git pack-objects --keep-true-parents \ > + --honor-pack-keep --assume-kept-packs-closed \ > + --keep-pack=pack-$base_pack.pack \ > + --non-empty --all \ > + --reflog --indexed-objects --delta-base-offset \ > + --stdout </dev/null >/dev/null > + ' > done > > # Measure pack loading with 10,000 packs.
On Thu, Jan 28, 2021 at 07:40:40PM -0800, Junio C Hamano wrote: > > diff --git a/t/perf/p5303-many-packs.sh b/t/perf/p5303-many-packs.sh > > index 277d22ec4b..85b077b72b 100755 > > --- a/t/perf/p5303-many-packs.sh > > +++ b/t/perf/p5303-many-packs.sh > > @@ -27,8 +27,11 @@ repack_into_n () { > > this construct: > > ... | > sed -n '1~5p' | > head -n "$1" | > ... > > which is a GNUism. Peff often says that very small population > actually run our perf suite, and this seems to corroborate the > conjecture. Oops. Looks like I was the one who introduced that. Nobody seems to have complained, so I'm somewhat tempted to leave it. But it would not be too hard to replace with perl, I think. -Peff
Jeff King <peff@peff.net> writes: > On Thu, Jan 28, 2021 at 07:40:40PM -0800, Junio C Hamano wrote: > >> > diff --git a/t/perf/p5303-many-packs.sh b/t/perf/p5303-many-packs.sh >> > index 277d22ec4b..85b077b72b 100755 >> > --- a/t/perf/p5303-many-packs.sh >> > +++ b/t/perf/p5303-many-packs.sh >> > @@ -27,8 +27,11 @@ repack_into_n () { >> >> this construct: >> >> ... | >> sed -n '1~5p' | >> head -n "$1" | >> ... >> >> which is a GNUism. Peff often says that very small population >> actually run our perf suite, and this seems to corroborate the >> conjecture. > > Oops. Looks like I was the one who introduced that. Nobody seems to have > complained, so I'm somewhat tempted to leave it. But it would not be too > hard to replace with perl, I think. Yeah, but would it be worth it? I am actually OK to say that you need GNU sed if you want to run perf. We already rely on GNU time to run perf tests, no?
On Fri, Jan 29, 2021 at 12:38:08PM -0800, Junio C Hamano wrote: > > Oops. Looks like I was the one who introduced that. Nobody seems to have > > complained, so I'm somewhat tempted to leave it. But it would not be too > > hard to replace with perl, I think. > > Yeah, but would it be worth it? I am actually OK to say that you > need GNU sed if you want to run perf. We already rely on GNU time > to run perf tests, no? True. This one is a little worse because it's subtle, and somebody might copy it unknowingly into the regular test suite. I am happy to leave it, or for you to pick up the patch I sent earlier (which I did verify produces identical output). -Peff
Jeff King <peff@peff.net> writes: > On Fri, Jan 29, 2021 at 12:38:08PM -0800, Junio C Hamano wrote: > >> > Oops. Looks like I was the one who introduced that. Nobody seems to have >> > complained, so I'm somewhat tempted to leave it. But it would not be too >> > hard to replace with perl, I think. >> >> Yeah, but would it be worth it? I am actually OK to say that you >> need GNU sed if you want to run perf. We already rely on GNU time >> to run perf tests, no? > > True. This one is a little worse because it's subtle, and somebody might > copy it unknowingly into the regular test suite. > > I am happy to leave it, or for you to pick up the patch I sent earlier > (which I did verify produces identical output). Yeah, I would be very unhappy if somebody copied-and-pasted it, but somehow I didn't think too many people moved code in that direction ;-) Will apply the portability fix, then. Thanks.
diff --git a/t/perf/p5303-many-packs.sh b/t/perf/p5303-many-packs.sh index 277d22ec4b..85b077b72b 100755 --- a/t/perf/p5303-many-packs.sh +++ b/t/perf/p5303-many-packs.sh @@ -27,8 +27,11 @@ repack_into_n () { >pushes && # create base packfile - head -n 1 pushes | - git pack-objects --delta-base-offset --revs staging/pack && + base_pack=$( + head -n 1 pushes | + git pack-objects --delta-base-offset --revs staging/pack + ) && + test_export base_pack && # and then incrementals between each pair of commits last= && @@ -87,6 +90,15 @@ do --reflog --indexed-objects --delta-base-offset \ --stdout </dev/null >/dev/null ' + + test_perf "repack with keep ($nr_packs)" ' + git pack-objects --keep-true-parents \ + --honor-pack-keep --assume-kept-packs-closed \ + --keep-pack=pack-$base_pack.pack \ + --non-empty --all \ + --reflog --indexed-objects --delta-base-offset \ + --stdout </dev/null >/dev/null + ' done # Measure pack loading with 10,000 packs.