Re: [PATCH 4/4] index-pack: Honor core.deltaBaseCacheLimit when resolving deltas

[Nicolas Pitre <nico@cam.org>]

On Sun, 13 Jul 2008, Shawn O. Pearce wrote:

> If we are trying to resolve deltas for a long delta chain composed
> of multi-megabyte objects we can easily run into requiring 500M+
> of memory to hold each object in the chain on the call stack while
> we recurse into the dependent objects and resolve them.
> 
> We now use a simple delta cache that discards objects near the
> bottom of the call stack first, as they are the most least recently
> used objects in this current delta chain.  If we recurse out of a
> chain we may find the base object is no longer available, as it was
> free'd to keep memory under the deltaBaseCacheLimit.  In such cases
> we must unpack the base object again, which will require recursing
> back to the root of the top of the delta chain as we released that
> root first.
> 
> Signed-off-by: Shawn O. Pearce <spearce@spearce.org>

OK now I see what the 'base' pointer I previously dismissed is really 
needed for.

But this patch is suboptimal as it actually recreate the same memory 
pressure, to a lesser degree, this series is meant to solve.  If you do:

> +		struct object_entry *obj = c->obj;
> +		void *raw = get_data_from_pack(obj);
> +		if (obj->type == OBJ_REF_DELTA || obj->type == OBJ_OFS_DELTA) {
> +			c->data = patch_delta(
> +				get_base_data(c->base), c->base->size,
> +				raw, obj->size,
> +				&c->size);
> +			free(raw);

What you actually do is to read the delta data in memory, then recurse 
down to read more delta data, then recurse down to read the base which 
might be yet more delta data in memory, etc. etc.  Only when you reach 
the bottom of the stack will you start resolving all those deltas in 
memory.  Instead, the check for a delta object should be done first, and 
if so then recursion for the base object be performed _before_ reading 
the currently wanted object data.  This way you won't have more than one 
delta buffer at any time in memory.


Nicolas