git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Taylor Blau <me@ttaylorr.com>
To: git@vger.kernel.org
Cc: peff@peff.net, dstolee@microsoft.com, gitster@pobox.com
Subject: [PATCH 1/3] builtin/commit-graph.c: support '--split[=<strategy>]'
Date: Thu, 30 Jan 2020 16:28:17 -0800	[thread overview]
Message-ID: <4f5bc19903f8a1f5b153b5665de378e743e12744.1580430057.git.me@ttaylorr.com> (raw)
In-Reply-To: <cover.1580430057.git.me@ttaylorr.com>

With '--split', the commit-graph machinery writes new commits in another
incremental commit-graph which is part of the existing chain, and
optionally decides to condense the chain into a single commit-graph.
This is done to ensure that the aysmptotic behavior of looking up a
commit in an incremental chain is dominated by the number of
incrementals in that chain. It can be controlled by the '--max-commits'
and '--size-multiple' options.

On occasion, callers may want to ensure that 'git commit-graph write
--split' always writes an incremental, and never spends effort
condensing the incremental chain [1]. Previously, this was possible by
passing '--size-multiple=0', but this no longer the case following
63020f175f (commit-graph: prefer default size_mult when given zero,
2020-01-02).

Reintroduce a less-magical variant of the above with a new pair of
arguments to '--split': '--split=no-merge' and '--split=merge-all'. When
'--split=no-merge' is given, the commit-graph machinery will never
condense an existing chain and will always write a new incremental.
Conversely, if '--split=merge-all' is given, any invocation including it
will always condense a chain if one exists.  If '--split' is given with
no arguments, it behaves as before and defers to '--size-multiple', and
so on.

[1]: This might occur when, for example, a server administrator running
some program after each push may want to ensure that each job runs
proportional in time to the size of the push, and does not "jump" when
the commit-graph machinery decides to trigger a merge.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 Documentation/git-commit-graph.txt | 18 +++++++++++-----
 builtin/commit-graph.c             | 33 ++++++++++++++++++++++++++----
 commit-graph.c                     | 19 +++++++++--------
 commit-graph.h                     |  7 +++++++
 t/t5324-split-commit-graph.sh      | 25 ++++++++++++++++++++++
 5 files changed, 85 insertions(+), 17 deletions(-)

diff --git a/Documentation/git-commit-graph.txt b/Documentation/git-commit-graph.txt
index 28d1fee505..8d61ba9f56 100644
--- a/Documentation/git-commit-graph.txt
+++ b/Documentation/git-commit-graph.txt
@@ -57,11 +57,19 @@ or `--stdin-packs`.)
 With the `--append` option, include all commits that are present in the
 existing commit-graph file.
 +
-With the `--split` option, write the commit-graph as a chain of multiple
-commit-graph files stored in `<dir>/info/commit-graphs`. The new commits
-not already in the commit-graph are added in a new "tip" file. This file
-is merged with the existing file if the following merge conditions are
-met:
+With the `--split[=<strategy>]` option, write the commit-graph as a
+chain of multiple commit-graph files stored in
+`<dir>/info/commit-graphs`. Commit-graph layers are merged based on the
+strategy and other splitting options. The new commits not already in the
+commit-graph are added in a new "tip" file. This file is merged with the
+existing file if the following merge conditions are met:
+* If `--split=merge-always` is specified, then a merge is always
+conducted, and the remaining options are ignored. Conversely, if
+`--split=no-merge` is specified, a merge is never performed, and the
+remaining options are ignored. A bare `--split` defers to the remaining
+options. (Note that merging a chain of commit graphs replaces the
+existing chain with a length-1 chain where the first and only
+incremental holds the entire graph).
 +
 * If `--size-multiple=<X>` is not specified, let `X` equal 2. If the new
 tip file would have `N` commits and the previous tip has `M` commits and
diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c
index de321c71ad..f03b46d627 100644
--- a/builtin/commit-graph.c
+++ b/builtin/commit-graph.c
@@ -9,7 +9,9 @@
 
 static char const * const builtin_commit_graph_usage[] = {
 	N_("git commit-graph verify [--object-dir <objdir>] [--shallow] [--[no-]progress]"),
-	N_("git commit-graph write [--object-dir <objdir>] [--append|--split] [--reachable|--stdin-packs|--stdin-commits] [--[no-]progress] <split options>"),
+	N_("git commit-graph write [--object-dir <objdir>] [--append] "
+	   "[--split[=<strategy>]] [--reachable|--stdin-packs|--stdin-commits] "
+	   "[--[no-]progress] <split options>"),
 	NULL
 };
 
@@ -19,7 +21,9 @@ static const char * const builtin_commit_graph_verify_usage[] = {
 };
 
 static const char * const builtin_commit_graph_write_usage[] = {
-	N_("git commit-graph write [--object-dir <objdir>] [--append|--split] [--reachable|--stdin-packs|--stdin-commits] [--[no-]progress] <split options>"),
+	N_("git commit-graph write [--object-dir <objdir>] [--append] "
+	   "[--split[=<strategy>]] [--reachable|--stdin-packs|--stdin-commits] "
+	   "[--[no-]progress] <split options>"),
 	NULL
 };
 
@@ -101,6 +105,25 @@ static int graph_verify(int argc, const char **argv)
 extern int read_replace_refs;
 static struct split_commit_graph_opts split_opts;
 
+static int write_option_parse_split(const struct option *opt, const char *arg,
+				    int unset)
+{
+	enum commit_graph_split_flags *flags = opt->value;
+
+	opts.split = 1;
+	if (!arg)
+		return 0;
+
+	if (!strcmp(arg, "merge-all"))
+		*flags = COMMIT_GRAPH_SPLIT_MERGE_REQUIRED;
+	else if (!strcmp(arg, "no-merge"))
+		*flags = COMMIT_GRAPH_SPLIT_MERGE_PROHIBITED;
+	else
+		die(_("unrecognized --split argument, %s"), arg);
+
+	return 0;
+}
+
 static int graph_write(int argc, const char **argv)
 {
 	struct string_list *pack_indexes = NULL;
@@ -123,8 +146,10 @@ static int graph_write(int argc, const char **argv)
 		OPT_BOOL(0, "append", &opts.append,
 			N_("include all commits already in the commit-graph file")),
 		OPT_BOOL(0, "progress", &opts.progress, N_("force progress reporting")),
-		OPT_BOOL(0, "split", &opts.split,
-			N_("allow writing an incremental commit-graph file")),
+		OPT_CALLBACK_F(0, "split", &split_opts.flags, NULL,
+			N_("allow writing an incremental commit-graph file"),
+			PARSE_OPT_OPTARG | PARSE_OPT_NONEG,
+			write_option_parse_split),
 		OPT_INTEGER(0, "max-commits", &split_opts.max_commits,
 			N_("maximum number of commits in a non-base split commit-graph")),
 		OPT_INTEGER(0, "size-multiple", &split_opts.size_multiple,
diff --git a/commit-graph.c b/commit-graph.c
index 6d34829f57..02e6ad9d1f 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -1565,15 +1565,18 @@ static void split_graph_merge_strategy(struct write_commit_graph_context *ctx)
 	num_commits = ctx->commits.nr;
 	ctx->num_commit_graphs_after = ctx->num_commit_graphs_before + 1;
 
-	while (g && (g->num_commits <= size_mult * num_commits ||
-		    (max_commits && num_commits > max_commits))) {
-		if (g->odb != ctx->odb)
-			break;
+	if (ctx->split_opts->flags != COMMIT_GRAPH_SPLIT_MERGE_PROHIBITED) {
+		while (g && (g->num_commits <= size_mult * num_commits ||
+			    (max_commits && num_commits > max_commits) ||
+			    (ctx->split_opts->flags == COMMIT_GRAPH_SPLIT_MERGE_REQUIRED))) {
+			if (g->odb != ctx->odb)
+				break;
 
-		num_commits += g->num_commits;
-		g = g->base_graph;
+			num_commits += g->num_commits;
+			g = g->base_graph;
 
-		ctx->num_commit_graphs_after--;
+			ctx->num_commit_graphs_after--;
+		}
 	}
 
 	ctx->new_base_graph = g;
@@ -1881,7 +1884,7 @@ int write_commit_graph(struct object_directory *odb,
 		goto cleanup;
 	}
 
-	if (!ctx->commits.nr)
+	if (!ctx->commits.nr && (!ctx->split || ctx->split_opts->flags != COMMIT_GRAPH_SPLIT_MERGE_REQUIRED))
 		goto cleanup;
 
 	if (ctx->split) {
diff --git a/commit-graph.h b/commit-graph.h
index 7d9fc9c16a..dadcc03808 100644
--- a/commit-graph.h
+++ b/commit-graph.h
@@ -84,10 +84,17 @@ enum commit_graph_write_flags {
 	COMMIT_GRAPH_WRITE_CHECK_OIDS = (1 << 3)
 };
 
+enum commit_graph_split_flags {
+	COMMIT_GRAPH_SPLIT_UNSPECIFIED      = 0,
+	COMMIT_GRAPH_SPLIT_MERGE_REQUIRED   = 1,
+	COMMIT_GRAPH_SPLIT_MERGE_PROHIBITED = 2
+};
+
 struct split_commit_graph_opts {
 	int size_multiple;
 	int max_commits;
 	timestamp_t expire_time;
+	enum commit_graph_split_flags flags;
 };
 
 /*
diff --git a/t/t5324-split-commit-graph.sh b/t/t5324-split-commit-graph.sh
index c24823431f..a165b48afe 100755
--- a/t/t5324-split-commit-graph.sh
+++ b/t/t5324-split-commit-graph.sh
@@ -344,4 +344,29 @@ test_expect_success 'split across alternate where alternate is not split' '
 	test_cmp commit-graph .git/objects/info/commit-graph
 '
 
+test_expect_success '--split=merge-all always merges incrementals' '
+	test_when_finished rm -rf a b c &&
+	rm -rf $graphdir $infodir/commit-graph &&
+	git reset --hard commits/10 &&
+	git rev-list -3 HEAD~4 >a &&
+	git rev-list -2 HEAD~2 >b &&
+	git rev-list -2 HEAD >c &&
+	git commit-graph write --split=no-merge --stdin-commits <a &&
+	git commit-graph write --split=no-merge --stdin-commits <b &&
+	test_line_count = 2 $graphdir/commit-graph-chain &&
+	git commit-graph write --split=merge-all --stdin-commits <c &&
+	test_line_count = 1 $graphdir/commit-graph-chain
+'
+
+test_expect_success '--split=no-merge always writes an incremental' '
+	test_when_finished rm -rf a b &&
+	rm -rf $graphdir &&
+	git reset --hard commits/2 &&
+	git rev-list HEAD~1 >a &&
+	git rev-list HEAD >b &&
+	git commit-graph write --split --stdin-commits <a &&
+	git commit-graph write --split=no-merge --stdin-commits <b &&
+	test_line_count = 2 $graphdir/commit-graph-chain
+'
+
 test_done
-- 
2.25.0.dirty


  reply	other threads:[~2020-01-31  0:28 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-31  0:28 [PATCH 0/3] builtin/commit-graph.c: new split/merge options Taylor Blau
2020-01-31  0:28 ` Taylor Blau [this message]
2020-01-31 14:19   ` [PATCH 1/3] builtin/commit-graph.c: support '--split[=<strategy>]' Derrick Stolee
2020-02-04  3:47     ` Taylor Blau
2020-01-31 19:27   ` Martin Ågren
2020-02-04  4:06     ` Taylor Blau
2020-02-06 19:15       ` Martin Ågren
2020-02-09 23:27         ` Taylor Blau
2020-01-31 23:34   ` SZEDER Gábor
2020-02-01 21:25     ` Johannes Schindelin
2020-02-03 10:47       ` SZEDER Gábor
2020-02-03 11:11         ` Jeff King
2020-02-04  3:58           ` Taylor Blau
2020-02-04 14:14             ` Jeff King
2020-02-04  3:59       ` Taylor Blau
2020-02-04  3:59     ` Taylor Blau
2020-01-31  0:28 ` [PATCH 2/3] builtin/commit-graph.c: introduce '--input=<source>' Taylor Blau
2020-01-31 14:40   ` Derrick Stolee
2020-02-04  4:21     ` Taylor Blau
2020-01-31 19:34   ` Martin Ågren
2020-02-04  4:51     ` Taylor Blau
2020-02-13 11:33       ` SZEDER Gábor
2020-02-13 11:48         ` SZEDER Gábor
2020-02-13 17:56           ` Taylor Blau
2020-01-31  0:28 ` [PATCH 3/3] builtin/commit-graph.c: support '--input=none' Taylor Blau
2020-01-31 14:40   ` Derrick Stolee
2020-01-31 19:45   ` Martin Ågren
2020-02-04  5:01     ` Taylor Blau
2020-01-31  0:32 ` [PATCH 0/3] builtin/commit-graph.c: new split/merge options Taylor Blau
2020-01-31 13:26   ` Derrick Stolee
2020-01-31 14:41 ` Derrick Stolee
2020-02-04 23:44 ` Junio C Hamano
2020-02-05  0:30   ` Taylor Blau
2020-02-05  0:28 ` [PATCH v2 " Taylor Blau
2020-02-05  0:28   ` [PATCH v2 1/3] builtin/commit-graph.c: support '--split[=<strategy>]' Taylor Blau
2020-02-06 19:41     ` Martin Ågren
2020-02-07 15:48       ` Derrick Stolee
2020-02-09 23:32         ` Taylor Blau
2020-02-12  6:03         ` Martin Ågren
2020-02-12 20:50           ` Taylor Blau
2020-02-09 23:30       ` Taylor Blau
2020-02-05  0:28   ` [PATCH v2 2/3] builtin/commit-graph.c: introduce '--input=<source>' Taylor Blau
2020-02-05  0:28   ` [PATCH v2 3/3] builtin/commit-graph.c: support '--input=none' Taylor Blau
2020-02-06 19:50     ` Martin Ågren
2020-02-09 23:32       ` Taylor Blau
2020-02-05 20:07   ` [PATCH v2 0/3] builtin/commit-graph.c: new split/merge options Junio C Hamano
2020-02-12  5:47 ` [PATCH v3 " Taylor Blau
2020-02-12  5:47   ` [PATCH v3 1/3] builtin/commit-graph.c: support '--split[=<strategy>]' Taylor Blau
2020-02-12  5:47   ` [PATCH v3 2/3] builtin/commit-graph.c: introduce '--input=<source>' Taylor Blau
2020-02-12  5:47   ` [PATCH v3 3/3] builtin/commit-graph.c: support '--input=none' Taylor Blau
2020-02-13 11:39     ` SZEDER Gábor
2020-02-13 12:31     ` SZEDER Gábor
2020-02-13 16:08       ` Junio C Hamano
2020-02-13 17:58         ` Taylor Blau
2020-02-13 17:56       ` Taylor Blau
2020-02-12 18:19   ` [PATCH v3 0/3] builtin/commit-graph.c: new split/merge options Junio C Hamano
2020-02-13 17:41     ` Taylor Blau
2020-02-17 18:24   ` Martin Ågren

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4f5bc19903f8a1f5b153b5665de378e743e12744.1580430057.git.me@ttaylorr.com \
    --to=me@ttaylorr.com \
    --cc=dstolee@microsoft.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).