Re: [PATCH] pack-objects.h: remove outdated pahole results

From: Jeff King <peff@peff.net>
Date: Jul 1, 2022, 18:16 UTC
Message-ID: <Yr85+tcexG2geaPP@coredump.intra.peff.net>
In-Reply-To: <1379af2e9d271b501ef3942398e7f159a9c77973.1656440978.git.me@ttaylorr.com>

On Tue, Jun 28, 2022 at 02:30:20PM -0400, Taylor Blau wrote:

Show 7 quoted lines

> Even though this comment was written in a good spirit, it is updated
> infrequently enough that is serves to confuse rather than to encourage
> contributors to update the appropriate values when the modify the
> definition of object_entry.
> 
> For that reason, eliminate the confusion by removing the comment
> altogether.

I agree the actual numbers aren't helping anybody. We _could_ leave a comment that says "we store a lot of these in memory; be careful of where and how you add new fields to avoid increasing the struct size". And then people can run "pahole" before and after their changes.

But then that is also true of other structs (like "struct object"), and we do not bother there. So it probably is fine not to annotate this specifically.

Speaking of which, I suspect quite a lot of memory could be saved if "pack-objects --revs" freed the object structs it allocates during its traversal. Unless we're generating bitmaps, I don't think they get used again after the initial packing list is generated. At peak you'd still be storing all of the object_entry structs alongside the objects as you finish the traversal, but it wouldn't overlap with any memory used for the delta search, and of course we'd be at that peak for a much smaller time.

Not a blocker for your patch obviously, but maybe a fun experiment in an adjacent area. Possibly even an ambitious #leftoverbits opportunity. :)

-Peff