git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Taylor Blau <me@ttaylorr.com>
To: Derrick Stolee <stolee@gmail.com>
Cc: "René Scharfe" <l.s.r@web.de>, "Git List" <git@vger.kernel.org>,
	"Junio C Hamano" <gitster@pobox.com>, "Jeff King" <peff@peff.net>
Subject: Re: [PATCH v2 0/5] packfile: use oidset for bad objects
Date: Sun, 12 Sep 2021 00:01:41 -0400	[thread overview]
Message-ID: <YT17pcvyq2nyyc3o@nand.local> (raw)
In-Reply-To: <4d76db59-e049-32c3-c4dd-ab7f70cbc20b@gmail.com>

On Sat, Sep 11, 2021 at 11:51:36PM -0400, Derrick Stolee wrote:
> On 9/11/21 10:29 PM, Taylor Blau wrote:
> > On Sat, Sep 11, 2021 at 09:51:04PM -0400, Taylor Blau wrote:
> >> On Sat, Sep 11, 2021 at 07:59:40PM -0400, Derrick Stolee wrote:
> >>> I initially hesitated to support the drop of
> >>> nth_midxed_pack_entry(), since it was designed with things
> >>> like midx bitmaps in mind (specifically, to also support
> >>> lex-order-to-stable-order conversions).
> >>
> >> I didn't know that nth_midxed_pack_entry was designed with either
> >> purpose in mind, since it predates midx bitmaps by quite a bit.
> >
> > Thinking on it more, I can imagine that you wrote this function
> > aspirationally envisioning something like MIDX bitmaps. And since you
> > and I discussed the design together quite a bit, I imagine that that's
> > the case ;-).
> >
> > But I agree that after reading this series again, that the inline-ing
> > suggested makes sense (and doesn't conflict with any series I have in
> > flight which don't add any new callers).
>
> I'm thinking more to my original design of the multi-pack-index.
> At that time, I was thinking about the possible integration
> with bitmaps based on my experience in other systems which used
> a stable object order to allow writing bitmaps asynchronously
> with respect to the multi-pack-index write and object packing.

Makes sense, and thank you for clarifying. After re-reading my first
email, I figured that this is what you must have been talking about,
which is why I felt like I should rephrase (hence the follow-up email).

> One thing that you did when first considering bitmaps over the
> multi-pack-index was to demonstrate that a stable object order
> is not required, which surprised and delighted me. It greatly
> reduced the complexity of the problem, and being able to inline
> this method is only one small fallout from that simplicity.

This was definitely a consequence of what I had observed from seeing
what was "slow" when running bitmaps in production at GitHub. There,
repacking a repository's objects all into one pack each time we ran our
automated background jobs far outpaced the amount of time we spent
generating bitmaps.

And with your and Peff's work on improving bitmap generation itself,
things are in a pretty good place. I do have some potential ideas for
future improvement, like a mode where bitmaps are only "fast forwarded",
meaning that new bitmaps are only added between the commits selected for
bitmapping in the previous round and the current reference tips.

I think things like that end up getting you pretty far, but it may be
interesting to come back eventually and revisit adding a stable object
order. In the meantime, though... ;)

Thanks,
Taylor

      reply	other threads:[~2021-09-12  4:01 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-11  7:50 [PATCH 0/3] packfile: use oidset for bad objects René Scharfe
2021-09-11  7:59 ` [PATCH 1/3] packfile: convert mark_bad_packed_object() to object_id René Scharfe
2021-09-11  8:00 ` [PATCH 2/3] packfile: convert has_packed_and_bad() " René Scharfe
2021-09-11  8:01 ` [PATCH 3/3] packfile: use oidset for bad objects René Scharfe
2021-09-11 14:26   ` Jeff King
2021-09-11 16:08     ` René Scharfe
2021-09-11 17:03       ` Jeff King
2021-09-11 17:16         ` René Scharfe
2021-09-11 14:27 ` [PATCH 0/3] " Jeff King
2021-09-11 16:08 ` [PATCH 4/3] midx: inline nth_midxed_pack_entry() René Scharfe
2021-09-11 17:03   ` René Scharfe
2021-09-11 17:07   ` Jeff King
2021-09-11 20:31     ` René Scharfe
2021-09-11 21:20       ` Jeff King
2021-09-11 23:39         ` René Scharfe
2021-09-11 20:31 ` [PATCH v2 0/5] packfile: use oidset for bad objects René Scharfe
2021-09-11 20:36   ` [PATCH 1/5] oidset: make oidset_size() an inline function René Scharfe
2021-09-11 20:39   ` [PATCH 2/5] midx: inline nth_midxed_pack_entry() René Scharfe
2021-09-11 20:40   ` [PATCH 3/5] packfile: convert mark_bad_packed_object() to object_id René Scharfe
2021-09-11 20:42   ` [PATCH v2 4/5] packfile: convert has_packed_and_bad() " René Scharfe
2021-09-11 20:43   ` [PATCH v2 5/5] packfile: use oidset for bad objects René Scharfe
2021-09-11 20:45   ` [PATCH v2 0/5] " René Scharfe
2021-09-11 21:22   ` Jeff King
2021-09-11 23:59   ` Derrick Stolee
2021-09-12  1:51     ` Taylor Blau
2021-09-12  2:29       ` Taylor Blau
2021-09-12  3:51         ` Derrick Stolee
2021-09-12  4:01           ` Taylor Blau [this message]

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=YT17pcvyq2nyyc3o@nand.local \
    --to=me@ttaylorr.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=l.s.r@web.de \
    --cc=peff@peff.net \
    --cc=stolee@gmail.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.
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).