Re: [PATCH v7 07/13] pack-objects: refer to delta objects by index instead of pointer
- From
Jeff King <peff@peff.net>
- Date
- Mar 30, 2018, 20:53 UTC
- Message-ID
- <20180330205346.GF14782@sigill.intra.peff.net>
- In-Reply-To
- <20180324063353.24722-8-pclouds@gmail.com>
On Sat, Mar 24, 2018 at 07:33:47AM +0100, Nguyễn Thái Ngọc Duy wrote:
Show 14 quoted lines
> These delta pointers always point to elements in the objects[] array > in packing_data struct. We can only hold maximum 4G of those objects > because the array size in nr_objects is uint32_t. We could use > uint32_t indexes to address these elements instead of pointers. On > 64-bit architecture (8 bytes per pointer) this would save 4 bytes per > pointer. > > Convert these delta pointers to indexes. Since we need to handle NULL > pointers as well, the index is shifted by one [1]. > > [1] This means we can only index 2^32-2 objects even though nr_objects > could contain 2^32-1 objects. It should not be a problem in > practice because when we grow objects[], nr_alloc would probably > blow up long before nr_objects hits the wall.
Hmm, that may be something we eventually fix. I suspect all of this code does some pretty horrible things as you approach 2^32 objects, though. I've never tried to make such a pack, but it may be within the realm of possibility. The .idx file would be 80+GB, but the packfile might not be much bigger if specially crafted.
I guess that's outside the realm of reasonable, though, so we can assume that nobody would _really_ want to do that anytime soon. And anything malicious would probably die long before this code triggers.
Show 5 quoted lines
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> > --- > builtin/pack-objects.c | 116 ++++++++++++++++++++++------------------- > pack-objects.h | 67 ++++++++++++++++++++++-- > 2 files changed, 124 insertions(+), 59 deletions(-)
The patch itself looks OK. This is one of the nicer ones, because it really doesn't involve any extra storage management, just some accessor functions.
-Peff