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

Re: [PATCH 1/1] fetch: set size_multiple in split_commit_graph_opts

From
Jeff King <peff@peff.net>
Date
Jan 6, 2020, 19:39 UTC
Message-ID
<20200106193903.GB971477@coredump.intra.peff.net>
In-Reply-To
<91d89356a20625d04af74d458c28b32445e760c1.1577981654.git.gitgitgadget@gmail.com>
On Thu, Jan 02, 2020 at 04:14:14PM +0000, Derrick Stolee via GitGitGadget wrote:
Show 27 quoted lines
> From: Derrick Stolee <dstolee@microsoft.com>
> 
> In 50f26bd ("fetch: add fetch.writeCommitGraph config setting",
> 2019-09-02), the fetch builtin added the capability to write a
> commit-graph using the "--split" feature. This feature creates
> multiple commit-graph files, and those can merge based on a set
> of "split options" including a size multiple. The default size
> multiple is 2, which intends to provide a log_2 N depth of the
> commit-graph chain where N is the number of commits.
> 
> However, I noticed during dogfooding that my commit-graph chains
> were becoming quite large when left only to builds by 'git fetch'.
> It turns out that in split_graph_merge_strategy(), we default the
> size_mult variable to 2 except we override it with the context's
> split_opts if they exist. In builtin/fetch.c, we create such a
> split_opts, but do not populate it with values.
> 
> This problem is due to two failures:
> 
>  1. It is unclear that we can add the flag COMMIT_GRAPH_WRITE_SPLIT
>     with a NULL split_opts.
>  2. If we have a non-NULL split_opts, then we override the default
>     values even if a zero value is given.
> 
> Correct both of these issues. First, do not override size_mult when
> the options provide a zero value. Second, stop creating a split_opts
> in the fetch builtin.

Thanks, the explanation and fix (both parts) look good to me, modulo the subject correction you already noted.

> ---
>  builtin/fetch.c | 4 +---
>  commit-graph.c  | 4 +++-
Is it worth covering this with a test?

I guess the non-fetch code paths for splitting already cover this pretty well, and this is just about managing to get the right number into the commit-graph code. So perhaps it isn't worth it.

-Peff
Previous: Derrick Stolee
Message 6 of 6 in “[PERF BUG] Fix size_mult option in fetch.writeCommitGraph”
  1. 0/1 [PERF BUG] Fix size_mult option in fetch.writeCommitGraphDerrick Stolee via GitGitGadget, Jan 2, 2020
  2. 1/1 fetch: set size_multiple in split_commit_graph_optsDerrick Stolee via GitGitGadget, Jan 2, 2020
  3. Derrick StoleeJan 2, 2020
  4. Junio C HamanoJan 2, 2020
  5. Derrick StoleeJan 3, 2020
  6. Jeff KingJan 6, 2020

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.