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

[PATCH v3 0/3] Performance improvements for repacking non-promisor objects

From
JTJonathan Tan <jonathantanmy@google.com>
Date
Dec 3, 2024, 21:52 UTC
Message-ID
<cover.1733262661.git.jonathantanmy@google.com>
In-Reply-To
<cover.1733170252.git.jonathantanmy@google.com>

Apparently I did not save in my text editor (and didn't notice because the code comment was still valid syntactically, so everything still compiled). Here's a version with the updated and correctly formatted code comment.

Jonathan Tan (3):
  index-pack --promisor: dedup before checking links
  index-pack --promisor: don't check blobs
  index-pack --promisor: also check commits' trees
 builtin/index-pack.c | 103 +++++++++++++++++++++++++++++--------------
 1 file changed, 71 insertions(+), 32 deletions(-)
Range-diff against v2:
1:  7ae21c921f = 1:  7ae21c921f index-pack --promisor: dedup before checking links
2:  5a63c9a5ca ! 2:  a1d2a20203 index-pack --promisor: don't check blobs
    @@ builtin/index-pack.c: static void record_outgoing_link(const struct object_id *o
     +static void maybe_record_name_entry(const struct name_entry *entry)
     +{
     +	/*
    -+	 * The benefit of doing this is as above (fetch speedup), but the drawback
    -+is that if the packfile to be indexed references a local blob directly
    -+(that is, not through a local tree), that local blob is in danger of
    -+being garbage collected. Such a situation may arise if we push local
    -+commits, including one with a change to a blob in the root tree,
    -+and then the server incorporates them into its main branch through a
    -+"rebase" or "squash" merge strategy, and then we fetch the new main
    -+branch from the server.
    -+
    -+This situation has not been observed yet - we have only noticed missing
    -+commits, not missing trees or blobs. (In fact, if it were believed that
    -+only missing commits are problematic, one could argue that we should
    -+also exclude trees during the outgoing link check; but it is safer to
    -+include them.)
    -+
    -+Due to the rarity of the situation (it has not been observed to happen
    -+in real life), and because the "penalty" in such a situation is merely
    -+to refetch the missing blob when it's needed, the tradeoff seems
    -+worth it.
    ++	 * Checking only trees here results in a significantly faster packfile
    ++	 * indexing, but the drawback is that if the packfile to be indexed
    ++	 * references a local blob only directly (that is, never through a
    ++	 * local tree), that local blob is in danger of being garbage
    ++	 * collected. Such a situation may arise if we push local commits,
    ++	 * including one with a change to a blob in the root tree, and then the
    ++	 * server incorporates them into its main branch through a "rebase" or
    ++	 * "squash" merge strategy, and then we fetch the new main branch from
    ++	 * the server.
    ++	 *
    ++	 * This situation has not been observed yet - we have only noticed
    ++	 * missing commits, not missing trees or blobs. (In fact, if it were
    ++	 * believed that only missing commits are problematic, one could argue
    ++	 * that we should also exclude trees during the outgoing link check;
    ++	 * but it is safer to include them.)
    ++	 *
    ++	 * Due to the rarity of the situation (it has not been observed to
    ++	 * happen in real life), and because the "penalty" in such a situation
    ++	 * is merely to refetch the missing blob when it's needed (and this
    ++	 * happens only once - when refetched, the blob goes into a promisor
    ++	 * pack, so it won't be GC-ed, the tradeoff seems worth it.
     +	*/
     +	if (S_ISDIR(entry->mode))
     +		record_outgoing_link(&entry->oid);
3:  8139325bf2 = 3:  f9f9969a8f index-pack --promisor: also check commits' trees
-- 
2.47.0.338.g60cca15819-goog
Previous: Jonathan TanNext: Jonathan Tan
Message 23 of 29 in “Performance improvements for repacking non-promisor objects”
  1. 0/3 Performance improvements for repacking non-promisor objectsJonathan Tan, Dec 2, 2024
  2. 1/3 index-pack: dedup first during outgoing link checkJonathan Tan, Dec 2, 2024
  3. Josh SteadmonDec 2, 2024
  4. 2/3 index-pack: no blobs during outgoing link checkJonathan Tan, Dec 2, 2024
  5. Patrick SteinhardtDec 3, 2024
  6. Jonathan TanDec 3, 2024
  7. Junio C HamanoDec 3, 2024
  8. 3/3 index-pack: commit tree during outgoing link checkJonathan Tan, Dec 2, 2024
  9. Junio C HamanoDec 3, 2024
  10. Jonathan TanDec 3, 2024
  11. Junio C HamanoDec 4, 2024
  12. Jonathan TanDec 9, 2024
  13. Junio C HamanoDec 9, 2024
  14. Josh SteadmonDec 2, 2024
  15. Junio C HamanoDec 3, 2024
  16. Junio C HamanoDec 3, 2024
  17. Junio C HamanoDec 3, 2024
  18. Junio C HamanoDec 3, 2024
  19. 0/3 Performance improvements for repacking non-promisor objectsJonathan Tan, Dec 3, 2024
  20. 1/3 index-pack --promisor: dedup before checking linksJonathan Tan, Dec 3, 2024
  21. 2/3 index-pack --promisor: don't check blobsJonathan Tan, Dec 3, 2024
  22. 3/3 index-pack --promisor: also check commits' treesJonathan Tan, Dec 3, 2024
  23. 0/3 Performance improvements for repacking non-promisor objectsJonathan Tan, Dec 3, 2024
  24. 1/3 index-pack --promisor: dedup before checking linksJonathan Tan, Dec 3, 2024
  25. Junio C HamanoDec 4, 2024
  26. 2/3 index-pack --promisor: don't check blobsJonathan Tan, Dec 3, 2024
  27. 3/3 index-pack --promisor: also check commits' treesJonathan Tan, Dec 3, 2024
  28. Junio C HamanoDec 4, 2024
  29. 4/3 index-pack: work around false positive use of uninitialized variableJunio C Hamano, Dec 4, 2024

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.