git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Junio C Hamano <gitster@pobox.com>
Date
Jul 6, 2015, 23:23 UTC
Message-ID
<xmqqvbdwyinx.fsf@gitster.dls.corp.google.com>
In-Reply-To
<xmqqr3on369x.fsf_-_@gitster.dls.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
Show 29 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.
>
> 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.

Previous: Junio C HamanoNext: Duy Nguyen
Message 8 of 17 in “nd/slim-index-pack-memory-usage update”
  1. 0/2 nd/slim-index-pack-memory-usage updateNguyễn Thái Ngọc Duy, Apr 18, 2015
  2. 1/2 index-pack: reduce object_entry size to save memoryNguyễn Thái Ngọc Duy, Apr 18, 2015
  3. 2/2 index-pack: kill union delta_base to save memoryNguyễn Thái Ngọc Duy, Apr 18, 2015
  4. Junio C HamanoJul 3, 2015
  5. Duy NguyenJul 3, 2015
  6. index-pack: fix overallocation of sorted_by_pos arrayJunio C Hamano, Jul 4, 2015
  7. index-pack: fix allocation of sorted_by_pos arrayJunio C Hamano, Jul 4, 2015
  8. Junio C HamanoJul 6, 2015
  9. Duy NguyenJul 7, 2015
  10. Junio C HamanoJul 7, 2015
  11. Jeff KingJul 7, 2015
  12. 1/2 index-pack: rename the field "type" to "in_pack_type"Nguyễn Thái Ngọc Duy, Jul 8, 2015
  13. 2/2 pack-objects: rename the field "type" to "real_type"Nguyễn Thái Ngọc Duy, Jul 8, 2015
  14. Jeff KingJul 8, 2015
  15. Duy NguyenJul 8, 2015
  16. Jeff KingJul 8, 2015
  17. Junio C HamanoJul 4, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.