Re: [PATCH 2/4] pack v4: add v4_size to struct delta_base_cache_entry

From: Duy Nguyen <pclouds@gmail.com>
Date: Sep 15, 2013, 07:35 UTC
Message-ID: <CACsJy8CrEwt+WiE9_0CU-TqMCJtNgzouc3e=ifQR=5r+GRXUPQ@mail.gmail.com>
In-Reply-To: <alpine.LFD.2.03.1309132351310.20709@syhkavp.arg>

On Sat, Sep 14, 2013 at 11:22 AM, Nicolas Pitre <nico@fluxnic.net> wrote:

Show 24 quoted lines

> The cache is currently updated by the caller.  The caller may ask for a
> copy of 2 entries from a base object, but that base object may itself
> copy those objects from somewhere else in a larger chunk.
>
> Let's consider this example:
>
> tree A
> ------
> 0 (0) copy 2 entries from tree B starting at entry 0
> 1 (2) copy 1 entry from tree B starting at entry 3
>
> tree B
> ------
> 0 (0) copy 6 entries from tree C starting at entry 0
> 1 (6) entry "foo.txt"
> 2 (7) entry "bar.txt"
>
> Right now, the code calls decode_entries() to decode 2 entries from tree
> B but those entries are part of a copy from tree C.  When that call
> returns, the cache is updated as if tree B entry #2 would start at
> offset 1 but this is wrong because offset 0 in tree B covers 6 entries
> and therefore offset 1 is for entry #6.
>
> So this needs a rethink.

I've given it some thought and see no simple/efficient way do it when 2+ depth is involved. Ideally tree A should refer to tree C directly for the first two entries, but in general we can't enforce that a copy sequence must refer to non-copy sequences only. Caching flattened tree B up until the 6th entry may help, but then there's no need to cache offsets anymore because we could just cache tree A.. -- Duy