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

Re: [PATCH v2 7/7] pack-objects: use mru list when iterating over packs

From
Junio C Hamano <gitster@pobox.com>
Date
Jul 29, 2016, 15:02 UTC
Message-ID
<xmqqr3acpjvo.fsf@gitster.mtv.corp.google.com>
In-Reply-To
<20160729054536.GA27343@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 17 quoted lines
> On Fri, Jul 29, 2016 at 12:15:24AM -0400, Jeff King wrote:
>
> But because this series switches the order of pack-lookup between
> objects, it is possible for us to find a `B` which is a delta against
> `A` in one pack, and then another copy of `A` which is a delta against
> another copy of `B` from another pack. We add both of the deltas to our
> packing list, but at write time when we try to write out all of the
> bases for `A`, we realize that whoops, we are recursing infinitely.
>
> As it turns out, Git actually handles this pretty well! Upon noticing
> the recursion, it breaks the delta chain and writes out one of the
> objects as a full base. This is due to Junio's f63c79d (pack-object:
> tolerate broken packs that have duplicated objects, 2011-11-16), though
> I think we later decided that duplicated objects were simply insane.
>
> So one option is to simply silence the warning, because the resulting
> pack is perfectly fine.
Thanks for a great analysis.

My gut feeling is that keeping the warning is preferred if possible, because f63c79db (pack-object: tolerate broken packs that have duplicated objects, 2011-11-16) was made as the last ditch effort to warn about the presence of the problem in the delta-base selection code without harming the users.

Show 6 quoted lines
> So it's possible that the resulting pack
> is not as small as it could be (i.e., we break the chain with a base
> object, but it's possible if we looked that we could have broken the
> chain by making a delta against an existing base object). So I wonder if
> it's possible to detect this case earlier, during the "can we reuse this
> delta" bits of check_object().

I'd let the issue simmer in my mind a bit for now, as I do not think of a neat trick offhand.

Previous: Jeff KingNext: Jeff King
Message 10 of 35 in “speed up pack-objects counting with many packs”
  1. 0/7 speed up pack-objects counting with many packsJeff King, Jul 29, 2016
  2. 1/7 t/perf: add tests for many-pack scenariosJeff King, Jul 29, 2016
  3. 2/7 sha1_file: drop free_pack_by_nameJeff King, Jul 29, 2016
  4. 3/7 add generic most-recently-used listJeff King, Jul 29, 2016
  5. 4/7 find_pack_entry: replace last_found_pack with MRU cacheJeff King, Jul 29, 2016
  6. 5/7 pack-objects: break out of want_object loop earlyJeff King, Jul 29, 2016
  7. 6/7 pack-objects: compute local/ignore_pack_keep earlyJeff King, Jul 29, 2016
  8. 7/7 pack-objects: use mru list when iterating over packsJeff King, Jul 29, 2016
  9. Jeff KingJul 29, 2016
  10. Junio C HamanoJul 29, 2016
  11. Jeff KingAug 8, 2016
  12. Junio C HamanoAug 8, 2016
  13. Jeff KingAug 8, 2016
  14. Junio C HamanoAug 8, 2016
  15. Jeff KingAug 9, 2016
  16. Jeff KingAug 9, 2016
  17. Junio C HamanoAug 9, 2016
  18. Junio C HamanoAug 9, 2016
  19. 0/2 pack-objects mruJeff King, Aug 10, 2016
  20. Junio C HamanoAug 10, 2016
  21. Jeff KingAug 11, 2016
  22. 1/2 pack-objects: break delta cycles before delta-search phaseJeff King, Aug 10, 2016
  23. Junio C HamanoAug 10, 2016
  24. Jeff KingAug 11, 2016
  25. 1/2 pack-objects: break delta cycles before delta-search phaseJeff King, Aug 11, 2016
  26. Jeff KingAug 11, 2016
  27. pack-objects mruJeff King, Aug 11, 2016
  28. 1/4 provide an initializer for "struct object_info"Jeff King, Aug 11, 2016
  29. 2/4 sha1_file: make packed_object_info publicJeff King, Aug 11, 2016
  30. 3/4 pack-objects: break delta cycles before delta-search phaseJeff King, Aug 11, 2016
  31. 4/4 pack-objects: use mru list when iterating over packsJeff King, Aug 11, 2016
  32. Jeff KingAug 11, 2016
  33. Junio C HamanoAug 11, 2016
  34. Jeff KingAug 11, 2016
  35. 2/2 pack-objects: use mru list when iterating over packsJeff King, Aug 10, 2016

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.