From: Duy Nguyen <pclouds@gmail.com>
To: Jeff King <peff@peff.net>
Cc: Elijah Newren <newren@gmail.com>, Git Mailing List <git@vger.kernel.org>
Subject: Re: 2.18.0 Regression: packing performance and effectiveness
Date: Thu, 19 Jul 2018 07:57:37 +0200 [thread overview]
Message-ID: <CACsJy8BNGboUb0F5NRR9tKj_P24j2ZgB48SCuLEphSHQAU2EUA@mail.gmail.com> (raw)
In-Reply-To: <20180719054424.GB23884@sigill.intra.peff.net>
On Thu, Jul 19, 2018 at 7:44 AM Jeff King <peff@peff.net> wrote:
>
> On Wed, Jul 18, 2018 at 03:51:08PM -0700, Elijah Newren wrote:
>
> > I had a user report some poor behavior of 'git gc --aggressive' on a
> > certain repo (which I sadly cannot share). Turns out that on this
> > repo, this operation takes about 60% longer and produces a pack
> > roughly twice the expected size.
> >
> > Naturally, bisecting takes a while but it points to this commit:
>
> Thanks for finding and bisecting.
>
> > To put some numbers behind this, I got on a very beefy box (40 cores,
> > 160GB RAM) and ran some comparisons:
> >
> > Version Pack (MB) MaxRSS(kB) Time (s)
> > ------- --------- ---------- --------
> > 2.17.0 5498 43513628 2494.85
> > 2.18.0 10531 40449596 4168.94
> > fix-v1 5509 42509784 2480.74
> > fiv-v2 5509 41644104 2468.25
> >
> > where fix-v1 and fix-v2 are different patches on top of git-2.18.0
> > that I'll follow up to this email with. The patches are just meant as
> > discussion starters. I'll add signoffs and proper commit messages if
> > folks actually want those fixes, but I suspect they're just starting
> > points for discussion.
>
> Hmm. So what's the mechanism causing the issue here? Looking at the
> patch from 0aca34e826 (pack-objects: shrink delta_size field in struct
> object_entry, 2018-04-14), it should _mostly_ be about how we store data
> in memory, and should not impact the deltas we find.
>
> But reading the patch and looking at your v2, it seems clear that this
> hunk is the culprit:
>
> @@ -2006,10 +2008,14 @@ static int try_delta(struct unpacked *trg, struct unpacked *src,
> delta_buf = create_delta(src->index, trg->data, trg_size, &delta_size, max_size);
> if (!delta_buf)
> return 0;
> + if (delta_size >= (1U << OE_DELTA_SIZE_BITS)) {
> + free(delta_buf);
> + return 0;
> + }
>
> So we're punting on large deltas, and presumably your test case has some
> files that are large but delta well (and we miss those opportunities,
> which wastes further time in the delta search and creates a lousy pack).
>
> I'm not sure why we need this hunk, though. Eventually we'd hit
> SET_DELTA_SIZE(), and I thought the point is that it should handle large
> sizes with some kind of fallback (the commit message even mentions an
> "overflow bit"). Looking at oe_set_delta_size(), though, I don't think
> the fallback would really work here; we'd hid the BUG().
The fallback is for existing on-disk deltas where we can actually get
the size from. For new deltas we have to store the size somewhere in
memory and we can't because this field's size is reduced.
I thought 2M was generous but I was obviously wrong. I'd like to see
the biggest delta size in this pack and whether it's still reasonable
to increase OE_DELTA_SIZE_BITS. But if it's very close to 4GB limit
then we could just store 64-bit delta size in a separate array.
--
Duy
next prev parent reply other threads:[~2018-07-19 5:58 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-07-18 22:51 2.18.0 Regression: packing performance and effectiveness Elijah Newren
2018-07-18 22:51 ` [RFC PATCH] fix-v1: revert "pack-objects: shrink delta_size field in struct object_entry" Elijah Newren
2018-07-18 22:51 ` [RFC PATCH] fix-v2: make OE_DELTA_SIZE_BITS a bit bigger Elijah Newren
2018-07-19 5:41 ` 2.18.0 Regression: packing performance and effectiveness Duy Nguyen
2018-07-19 5:49 ` Jeff King
2018-07-19 15:27 ` Elijah Newren
2018-07-19 15:43 ` Duy Nguyen
2018-07-19 5:44 ` Jeff King
2018-07-19 5:57 ` Duy Nguyen [this message]
2018-07-19 15:16 ` Duy Nguyen
2018-07-19 16:42 ` Elijah Newren
2018-07-19 17:23 ` Jeff King
2018-07-19 17:31 ` Duy Nguyen
2018-07-19 18:24 ` Duy Nguyen
2018-07-19 19:17 ` Jeff King
2018-07-19 23:11 ` Elijah Newren
2018-07-20 5:28 ` Jeff King
2018-07-20 5:30 ` Jeff King
2018-07-20 5:47 ` Duy Nguyen
2018-07-20 17:21 ` Elijah Newren
2018-07-19 17:04 ` Jeff King
2018-07-19 19:25 ` Junio C Hamano
2018-07-19 19:27 ` Junio C Hamano
2018-07-20 15:39 ` [PATCH] pack-objects: fix performance issues on packing large deltas Nguyễn Thái Ngọc Duy
2018-07-20 17:40 ` Jeff King
2018-07-21 4:23 ` Duy Nguyen
2018-07-23 21:37 ` Jeff King
2018-07-20 17:43 ` Elijah Newren
2018-07-20 23:52 ` Elijah Newren
2018-07-21 4:07 ` Duy Nguyen
2018-07-21 7:08 ` Duy Nguyen
2018-07-21 4:47 ` Duy Nguyen
2018-07-21 6:56 ` Elijah Newren
2018-07-21 7:14 ` Duy Nguyen
2018-07-22 6:22 ` Elijah Newren
2018-07-22 6:49 ` Duy Nguyen
2018-07-23 12:34 ` Elijah Newren
2018-07-23 15:50 ` Duy Nguyen
2018-07-22 8:04 ` [PATCH v2] " Nguyễn Thái Ngọc Duy
2018-07-23 18:04 ` Junio C Hamano
2018-07-23 18:38 ` Duy Nguyen
2018-07-23 18:49 ` Duy Nguyen
2018-07-23 21:30 ` Jeff King
2018-07-26 8:12 ` Johannes Sixt
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=CACsJy8BNGboUb0F5NRR9tKj_P24j2ZgB48SCuLEphSHQAU2EUA@mail.gmail.com \
--to=pclouds@gmail.com \
--cc=git@vger.kernel.org \
--cc=newren@gmail.com \
--cc=peff@peff.net \
/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.
Code repositories for project(s) associated with this public 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).