Re: [PATCH v4 0/7] merge-ort: implement support for packing objects together

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 20, 2023, 16:53 UTC
Message-ID: <xmqqpm19s0a0.fsf@gitster.g>
In-Reply-To: <20231020072941.GD1642714@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 6 quoted lines

>   - Having not worked with the bulk-checkin code much before, I thought
>     it only put in one blob per pack, and thus you'd have to teach it to
>     handle multiple objects, too. But fortunately I was wrong, and it
>     handles this case already. (I mention this mainly because we had
>     talked about it off-list a few weeks ago, and some of my confusion
>     may have seeped into my comments then).

You probably had it mixed up with a real limitation, i.e., "unlike fast-import that considers the immediate previous object could be a good delta base, this code does not even attempt any deltification" ;-)

Show 5 quoted lines

>   - I did briefly wonder if we need this SOURCE abstraction at all. The
>     file source assumes we can seek() and read(), so it must be a
>     regular file. We could probably just mmap() it and treat it as
>     in-core, too (this is what index_fd() and index_path() do under the
>     hood!).

Ahhhh, I've never thought of that one. I actually was hoping that we can add an abstraction layer that can be reused elsewhere and everywhere (not limited to the bulk-checkin codepath for its source material), which might help folks who want to refactor Git enough to allow them plug $CORP specific goodies like virtual filesystem layer ;-).

>     But it would probably be a disservice to any platforms that
>     do not support mmap, as the fallback is to read into a full buffer
>     (and the whole point of the bulk checkin code is to avoid that).

Yes.  The result was a pleasant read.

Thanks, both.