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

Re: [RFC PATCH 10/10] pack-objects: improve partial packfile reuse

From
Junio C Hamano <gitster@pobox.com>
Date
Oct 12, 2019, 00:04 UTC
Message-ID
<xmqqsgnyg76d.fsf@gitster-ct.c.googlers.com>
In-Reply-To
<20191011180125.GA20601@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 7 quoted lines
> The current code does so by creating a new entry in the reused_chunks
> array. In the worst case that can grow to have the same number of
> entries as we have objects. So this code was an attempt to pad the
> header of a shrunken entry to keep it the same size. I don't remember
> all the problems we ran into with that, but in the end we found that it
> didn't actually help much, because in practice we don't end up with a
> lot of chunks anyway.

Hmm, I am kind of surprised that the decoding side allowed such a padding.

> For instance, packing torvalds/linux on our servers just now reused 6.5M
> objects, but only needed ~50k chunks.
OK, that's a good argument for not trying to optimize.
Show 27 quoted lines
> I think the original code may simply have been buggy and nobody noticed.
> Here's what I wrote when this line was added in our fork:
>
>       pack-objects: check reused pack bitmap for duplicate objects
>   
>       If a client both asks for a tag by sha1 and specifies
>       "include-tag", we may end up including the tag in the reuse
>       bitmap (due to the first thing), and then later adding it to
>       the packlist (due to the second). This results in duplicate
>       objects in the pack, which git chokes on. We should notice
>       that we are already including it when doing the include-tag
>       portion, and avoid adding it to the packlist.
>   
>       The simplest place to fix this is right in add_ref_tag,
>       where we could avoid peeling the tag at all if we know that
>       we are already including it. However, I've pushed the check
>       instead into have_duplicate_entry. This fixes not only this
>       case, but also means that we cannot have any similar
>       problems lurking in other code.
>   
>       No tests, because git does not actually exhibit this "ask
>       for it and also include-tag" behavior. We do one or the
>       other on clone, depending on whether --single-branch is set.
>       However, libgit2 does both.
>
> I'm not sure why we didn't notice it with the older reuse code. It may
> simply have been that it hardly ever triggered on our servers.
Impressed by the careful thinking here.
Thanks.
Previous: Jonathan TanNext: Jeff King
Message 27 of 31 in “Rewrite packfile reuse code”
  1. 00/10 Rewrite packfile reuse codeChristian Couder, Sep 13, 2019
  2. 01/10 builtin/pack-objects: report reused packfile objectsChristian Couder, Sep 13, 2019
  3. 02/10 packfile: expose get_delta_base()Christian Couder, Sep 13, 2019
  4. 06/10 pack-bitmap: introduce bitmap_walk_contains()Christian Couder, Sep 13, 2019
  5. 04/10 ewah/bitmap: always allocate 2 more wordsChristian Couder, Sep 13, 2019
  6. Jonathan TanOct 10, 2019
  7. Christian CouderOct 11, 2019
  8. Jeff KingOct 11, 2019
  9. 07/10 csum-file: introduce hashfile_total()Christian Couder, Sep 13, 2019
  10. 08/10 pack-objects: introduce pack.allowPackReuseChristian Couder, Sep 13, 2019
  11. Junio C HamanoSep 13, 2019
  12. 03/10 ewah/bitmap: introduce bitmap_word_alloc()Christian Couder, Sep 13, 2019
  13. 05/10 pack-bitmap: don't rely on bitmap_git->reuse_objectsChristian Couder, Sep 13, 2019
  14. Jonathan TanOct 10, 2019
  15. Christian CouderOct 11, 2019
  16. 10/10 pack-objects: improve partial packfile reuseChristian Couder, Sep 13, 2019
  17. Junio C HamanoSep 13, 2019
  18. Jeff KingSep 14, 2019
  19. Junio C HamanoSep 14, 2019
  20. Jeff KingOct 2, 2019
  21. Junio C HamanoOct 3, 2019
  22. Christian CouderOct 3, 2019
  23. Jonathan TanOct 10, 2019
  24. Christian CouderOct 11, 2019
  25. Jeff KingOct 11, 2019
  26. Jonathan TanOct 11, 2019
  27. Junio C HamanoOct 12, 2019
  28. Jeff KingOct 13, 2019
  29. Junio C HamanoOct 17, 2019
  30. Jeff KingOct 17, 2019
  31. 09/10 builtin/pack-objects: introduce obj_is_packed()Christian Couder, Sep 13, 2019

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.