Re: [PATCH] index-pack: fix allocation of sorted_by_pos array

From: Duy Nguyen <pclouds@gmail.com>
Date: Jul 7, 2015, 00:36 UTC
Message-ID: <CACsJy8CsUu1zEnah9Ah3tQxk8N-xPpOBuV5TpQ4EB6+nyiDW3g@mail.gmail.com>
In-Reply-To: <xmqqr3on369x.fsf_-_@gitster.dls.corp.google.com>

On Sun, Jul 5, 2015 at 5:30 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 11 quoted lines

> When c6458e60 (index-pack: kill union delta_base to save memory,
> 2015-04-18) attempted to reduce the memory footprint of index-pack,
> one of the key thing it did was to keep track of ref-deltas and
> ofs-deltas separately.
>
> In fix_unresolved_deltas(), however it forgot that it now wants to
> look only at ref deltas in one place.  The code allocated an array
> for nr_unresolved, which is sum of number of ref- and ofs-deltas
> minus nr_resolved, which may be larger or smaller than the number
> ref-deltas.  Depending on nr_resolved, this was either under or over
> allocating.

It's either that or we could put back "if (real_type != OBJ_REF_DELTA) continue;" in the sorted_by_pos population loop. Resolved deltas can't have real_type == OBJ_REF_DELTA, so if we allocate nr_unresolved, it's guaranteed over-allocation, never under-allocation. But I guess your approach would make the code easier to read.

I keep tripping over this "real_type vs type" in this code. What do you think about renaming "type" field to "in_pack_type" and "real_type" to "canon_type" (or "final_type")? "Real" does not really say anything in this context..

-- 
Duy