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
Junio C Hamano <gitster@pobox.com>
Date
Jan 2, 2020, 21:49 UTC
Message-ID
<xmqq7e29edfd.fsf@gitster-ct.c.googlers.com>
In-Reply-To
<91d89356a20625d04af74d458c28b32445e760c1.1577981654.git.gitgitgadget@gmail.com>
"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 10 quoted lines
> 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.

OK, so there is the hardcoded default 2 in the code, and split_opts structure *can* override it, but 0 in the field of the structure is meant to signal "no, I do not have any value to override the default", not "I do want to set the multiple to 0"?

Makes sense.  Will queue.
Thanks.
Show 42 quoted lines
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---
>  builtin/fetch.c | 4 +---
>  commit-graph.c  | 4 +++-
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index f8765b385b..b4c6d921d0 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -1866,15 +1866,13 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
>  	    (fetch_write_commit_graph < 0 &&
>  	     the_repository->settings.fetch_write_commit_graph)) {
>  		int commit_graph_flags = COMMIT_GRAPH_WRITE_SPLIT;
> -		struct split_commit_graph_opts split_opts;
> -		memset(&split_opts, 0, sizeof(struct split_commit_graph_opts));
>  
>  		if (progress)
>  			commit_graph_flags |= COMMIT_GRAPH_WRITE_PROGRESS;
>  
>  		write_commit_graph_reachable(get_object_directory(),
>  					     commit_graph_flags,
> -					     &split_opts);
> +					     NULL);
>  	}
>  
>  	close_object_store(the_repository->objects);
> diff --git a/commit-graph.c b/commit-graph.c
> index e771394aff..b205e65ed1 100644
> --- a/commit-graph.c
> +++ b/commit-graph.c
> @@ -1542,7 +1542,9 @@ static void split_graph_merge_strategy(struct write_commit_graph_context *ctx)
>  
>  	if (ctx->split_opts) {
>  		max_commits = ctx->split_opts->max_commits;
> -		size_mult = ctx->split_opts->size_multiple;
> +
> +		if (ctx->split_opts->size_multiple)
> +			size_mult = ctx->split_opts->size_multiple;
>  	}
>  
>  	g = ctx->r->objects->commit_graph;
Previous: Derrick StoleeNext: Derrick Stolee
Message 4 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.