From: Taylor Blau <email@example.com> To: Jeff King <firstname.lastname@example.org> Cc: "Rasmus Villemoes" <email@example.com>, "Git Mailing List" <firstname.lastname@example.org>, "Stefan Beller" <email@example.com>, "Nguyễn Thái Ngọc Duy" <firstname.lastname@example.org> Subject: Re: [PATCH] packfile: actually set approximate_object_count_valid Date: Thu, 17 Sep 2020 12:53:00 -0400 [thread overview] Message-ID: <20200917165300.GA15187@nand.local> (raw) In-Reply-To: <20200917164743.GA3731633@coredump.intra.peff.net> On Thu, Sep 17, 2020 at 12:47:43PM -0400, Jeff King wrote: > So here it is wrapped up as a patch. I think it's worth fixing (as > opposed to dropping the unused flag code). Thanks for finding it. Yup, after reading the patch and performance timings below, I agree that this is worth fixing and keeping instead of dropping. > It doesn't help at all when we have 1 pack (5303.4), but we get a 10% > speedup when there are 1000 packs (5303.12). That's a modest speedup for > a case that's already slow and we'd hope to avoid in general (note how > slow it is even after, because we have to look in each of those packs > for abbreviations). But it's a one-line change that clearly matches the > original intent, so it seems worth doing. Excellent. > The included perf test may also be useful for keeping an eye on any > regressions in the overall abbreviation code. > > Reported-by: Rasmus Villemoes <email@example.com> > Signed-off-by: Jeff King <firstname.lastname@example.org> > --- > packfile.c | 1 + > t/perf/p5303-many-packs.sh | 4 ++++ > 2 files changed, 5 insertions(+) > > diff --git a/packfile.c b/packfile.c > index 9ef27508f2..e69012e7f2 100644 > --- a/packfile.c > +++ b/packfile.c > @@ -923,6 +923,7 @@ unsigned long repo_approximate_object_count(struct repository *r) > count += p->num_objects; > } > r->objects->approximate_object_count = count; > + r->objects->approximate_object_count_valid = 1; > } > return r->objects->approximate_object_count; > } > diff --git a/t/perf/p5303-many-packs.sh b/t/perf/p5303-many-packs.sh > index 7ee791669a..f4c2ab0584 100755 > --- a/t/perf/p5303-many-packs.sh > +++ b/t/perf/p5303-many-packs.sh > @@ -73,6 +73,10 @@ do > git rev-list --objects --all >/dev/null > ' > > + test_perf "abbrev-commit ($nr_packs)" ' > + git rev-list --abbrev-commit HEAD >/dev/null > + ' > + > # This simulates the interesting part of the repack, which is the > # actual pack generation, without smudging the on-disk setup > # between trials. > -- > 220.127.116.112.gdd163d6eb1 Looks all very good to me. Thanks. Thanks, Taylor
next prev parent reply other threads:[~2020-09-17 16:53 UTC|newest] Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-09-17 8:20 approximate_object_count_valid never set? Rasmus Villemoes 2020-09-17 11:58 ` Ævar Arnfjörð Bjarmason 2020-09-17 12:53 ` Jeff King 2020-09-17 16:47 ` [PATCH] packfile: actually set approximate_object_count_valid Jeff King 2020-09-17 16:53 ` Taylor Blau [this message] 2020-09-17 18:26 ` Junio C Hamano
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 List information: http://vger.kernel.org/majordomo-info.html * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200917165300.GA15187@nand.local \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --subject='Re: [PATCH] packfile: actually set approximate_object_count_valid' \ /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
Code repositories for project(s) associated with this inbox: https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).