# [PATCH] pack-objects: emit base before delta. 4 messages from 2005-07-01 to 2005-07-01. Participants: Junio C Hamano, Linus Torvalds. Thread: https://gitlist.dev/t/1066 ## Junio C Hamano, 2005-07-01 05:58 Subject: [PATCH] pack-objects: emit base before delta. Message-ID: <7vbr5nxe38.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vbr5nxe38.fsf%40assigned-by-dhcp.cox.net ``` This micro-optimizes the order of objects in a pack. By emitting base objects before deltified ones, unpack-objects do not keep items on delta_list. I think it is prudent to keep the add_delta_to_list() logic in unpack-objects, so this commit only changes the packing side. Signed-off-by: Junio C Hamano --- pack-objects.c | 8 +++++--- 1 files changed, 5 insertions(+), 3 deletions(-) 9be15981d7038f1d9d0b105f45760987584e60a6 diff --git a/pack-objects.c b/pack-objects.c --- a/pack-objects.c +++ b/pack-objects.c @@ -114,11 +114,13 @@ static unsigned long write_one(struct sh * if it is written already. */ return offset; - e->offset = offset; - offset += write_object(f, e); - /* if we are delitified, write out its base object. */ + + /* if we are delitified, write out its base objects first */ if (e->delta) offset = write_one(f, e->delta, offset); + + e->offset = offset; + offset += write_object(f, e); return offset; } ------------ ``` ## Junio C Hamano, 2005-07-01 06:18 Subject: Re: [PATCH] pack-objects: emit base before delta. Message-ID: <7v64vvxd5x.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7v64vvxd5x.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <7vbr5nxe38.fsf@assigned-by-dhcp.cox.net> ``` >>>>> "JCH" == Junio C Hamano writes: JCH> This micro-optimizes the order of objects in a pack. By JCH> emitting base objects before deltified ones, unpack-objects do JCH> not keep items on delta_list. Oops; "does not have to keep items" is what I meant. ``` ## Linus Torvalds, 2005-07-01 15:28 Subject: Re: [PATCH] pack-objects: emit base before delta. Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.58.0507010821140.14331%40ppc970.osdl.org In-Reply-To: <7vbr5nxe38.fsf@assigned-by-dhcp.cox.net> ``` On Thu, 30 Jun 2005, Junio C Hamano wrote: > > This micro-optimizes the order of objects in a pack. By > emitting base objects before deltified ones, unpack-objects do > not keep items on delta_list. So I like this from an unpacking standpoint, but at the same time I don't actually think it's correct from an access pattern standpoint. When using a pack file as a run-time object store, the current packing order means that we generally traverse the pack-file in a nice forward direction. We don't jump backwards in the file very much, which should be good both for CPU and disk caches (both of them tend to have prefetch logic that often works better for nice access patterns). So I was actually pretty happy with the fact that we packed "optimally" in this sense: if the object was an important delta (ie an early one), we'd basically end up always walking forward until we hit the object it was a delta against. I dunno. Maybe it doesn't matter. Our other heuristics to pack recent objects before later one might mean that we tend to have mainly backwards-going deltas (both in history _and_ in pack file layout). The expense of keeping track of delta objects isn't that high, and the "pending delta" logic in unpack-objects isn't that complicated, so .. Do you have some other reason you want to do this? Linus ``` ## Junio C Hamano, 2005-07-01 15:40 Subject: Re: [PATCH] pack-objects: emit base before delta. Message-ID: <7v1x6iilgj.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7v1x6iilgj.fsf%40assigned-by-dhcp.cox.net In-Reply-To: ``` >>>>> "LT" == Linus Torvalds writes: LT> So I like this from an unpacking standpoint, but at the same LT> time I don't actually think it's correct from an access LT> pattern standpoint. My faulty logic, when I did the patch, was "we would jump around on random access anyway, so forcing the runtime to go backwards even when we know that these 40 objects are followed in this exact order to resolve delta is also OK." It is not. Please drop the patch. ```