From: Junio C Hamano <gitster@pobox•com>
To: Git Mailing List <git@vger•kernel.org>
Cc: Duy Nguyen <pclouds@gmail•com>
Subject: Re: [PATCH] index-pack: fix allocation of sorted_by_pos array
Date: Mon, 06 Jul 2015 16:23:46 -0700 [thread overview]
Message-ID: <xmqqvbdwyinx.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <xmqqr3on369x.fsf_-_@gitster.dls.corp.google.com> (Junio C. Hamano's message of "Sat, 04 Jul 2015 15:30:18 -0700")
Junio C Hamano <gitster@pobox•com> writes:
> 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.
>
> Also, the old code before this change had to use 'i' and 'n' because
> some of the things we see in the (old) deltas[] array we scanned
> with 'i' would not make it into the sorted_by_pos[] array in the old
> world order, but now because you have only ref delta in a separate
> ref_deltas[] array, they increment lock&step. We no longer need
> separate variables. And most importantly, we shouldn't pass the
> nr_unresolved parameter, as this number does not play a role in the
> working of this helper function.
>
> Helped-by: Nguyễn Thái Ngọc Duy <pclouds@gmail•com>
> Signed-off-by: Junio C Hamano <gitster@pobox•com>
> ---
>
> * This time, correcting the analysis; the previous one claimed that
> this was not a bug but just an overallocation. It indeed is a bug
> that uses an unrelated value that may or may not be sufficiently
> large to hold the whole thing, I think.
I think this one as a real "crash hotfix" must be in the upcoming
release (the obvious alternative is to revert the series with
c6458e60 which I really want to avoid).
As Eric's "worktree add" would need some more time to solidify,
let's delay the -rc2 and later by a few weeks.
next prev parent reply other threads:[~2015-07-06 23:23 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-04-18 10:47 [PATCH 0/2] nd/slim-index-pack-memory-usage update Nguyễn Thái Ngọc Duy
2015-04-18 10:47 ` [PATCH 1/2] index-pack: reduce object_entry size to save memory Nguyễn Thái Ngọc Duy
2015-04-18 10:47 ` [PATCH 2/2] index-pack: kill union delta_base " Nguyễn Thái Ngọc Duy
2015-07-03 16:51 ` Junio C Hamano
2015-07-03 18:29 ` Duy Nguyen
2015-07-04 1:21 ` [PATCH] index-pack: fix overallocation of sorted_by_pos array Junio C Hamano
2015-07-04 22:30 ` [PATCH] index-pack: fix allocation " Junio C Hamano
2015-07-06 23:23 ` Junio C Hamano [this message]
2015-07-07 0:36 ` Duy Nguyen
2015-07-07 15:49 ` Junio C Hamano
2015-07-07 16:06 ` Jeff King
2015-07-08 11:56 ` [PATCH 1/2] index-pack: rename the field "type" to "in_pack_type" Nguyễn Thái Ngọc Duy
2015-07-08 11:56 ` [PATCH 2/2] pack-objects: rename the field "type" to "real_type" Nguyễn Thái Ngọc Duy
2015-07-08 13:47 ` Jeff King
2015-07-08 13:57 ` Duy Nguyen
2015-07-08 14:11 ` Jeff King
2015-07-04 22:24 ` [PATCH 2/2] index-pack: kill union delta_base to save memory Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqvbdwyinx.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=pclouds@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox