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

[PATCH v2 4/6] commit-graph: compute generations separately

From
Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>
Date
Feb 2, 2021, 03:01 UTC
Message-ID
<05248ff222fe7f2a7976e7a7cf2ee272eec6edbb.1612234883.git.gitgitgadget@gmail.com>
In-Reply-To
<pull.850.v2.git.1612234883.gitgitgadget@gmail.com>
From: Derrick Stolee <dstolee@microsoft.com>

The compute_generation_numbers() method was introduced by 3258c663 (commit-graph: compute generation numbers, 2018-05-01) to compute what is now known as "topological levels". These are still stored in the commit-graph file for compatibility sake while c1a09119 (commit-graph: implement corrected commit date, 2021-01-16) updated the method to also compute the new version of generation numbers: corrected commit date.

It makes sense why these are grouped. They perform very similar walks of the necessary commits and compute similar maximums over each parent. However, having these two together conflates them in subtle ways that is hard to separate.

In particular, the topo_level slab is used to store the topological levels in all cases, but the commit_graph_data_at(c)->generation member stores different values depending on the state of the existing commit-graph file.

* If the existing commit-graph file has a "GDAT" chunk, then these
  values represent corrected commit dates.
* If the existing commit-graph file doesn't have a "GDAT" chunk, then
  these values are actually the topological levels.

This issue only occurs only when upgrading an existing commit-graph file into one that has the "GDAT" chunk. The current change does not resolve this upgrade problem, but splitting the implementation into two pieces here helps with that process, which will follow in the next change.

The important thing this helps with is the case where the num_generation_data_overflows was being incremented incorrectly, triggering a write of the overflow chunk.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 commit-graph.c | 70 ++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 56 insertions(+), 14 deletions(-)
diff --git a/commit-graph.c b/commit-graph.c
index b3f7c3bbcb3..2790f70d113 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -1446,27 +1446,24 @@ static void close_reachable(struct write_commit_graph_context *ctx)
 	stop_progress(&ctx->progress);
 }
 
-static void compute_generation_numbers(struct write_commit_graph_context *ctx)
+static void compute_topological_levels(struct write_commit_graph_context *ctx)
 {
 	int i;
 	struct commit_list *list = NULL;
 
 	if (ctx->report_progress)
 		ctx->progress = start_delayed_progress(
-					_("Computing commit graph generation numbers"),
+					_("Computing commit graph topological levels"),
 					ctx->commits.nr);
 	for (i = 0; i < ctx->commits.nr; i++) {
 		struct commit *c = ctx->commits.list[i];
 		uint32_t level;
-		timestamp_t corrected_commit_date;
 
 		repo_parse_commit(ctx->r, c);
 		level = *topo_level_slab_at(ctx->topo_levels, c);
-		corrected_commit_date = commit_graph_data_at(c)->generation;
 
 		display_progress(ctx->progress, i + 1);
-		if (level != GENERATION_NUMBER_ZERO &&
-		    corrected_commit_date != GENERATION_NUMBER_ZERO)
+		if (level != GENERATION_NUMBER_ZERO)
 			continue;
 
 		commit_list_insert(c, &list);
@@ -1475,15 +1472,12 @@ static void compute_generation_numbers(struct write_commit_graph_context *ctx)
 			struct commit_list *parent;
 			int all_parents_computed = 1;
 			uint32_t max_level = 0;
-			timestamp_t max_corrected_commit_date = 0;
 
 			for (parent = current->parents; parent; parent = parent->next) {
 				repo_parse_commit(ctx->r, parent->item);
 				level = *topo_level_slab_at(ctx->topo_levels, parent->item);
-				corrected_commit_date = commit_graph_data_at(parent->item)->generation;
 
-				if (level == GENERATION_NUMBER_ZERO ||
-				    corrected_commit_date == GENERATION_NUMBER_ZERO) {
+				if (level == GENERATION_NUMBER_ZERO) {
 					all_parents_computed = 0;
 					commit_list_insert(parent->item, &list);
 					break;
@@ -1491,9 +1485,6 @@ static void compute_generation_numbers(struct write_commit_graph_context *ctx)
 
 				if (level > max_level)
 					max_level = level;
-
-				if (corrected_commit_date > max_corrected_commit_date)
-					max_corrected_commit_date = corrected_commit_date;
 			}
 
 			if (all_parents_computed) {
@@ -1502,6 +1493,55 @@ static void compute_generation_numbers(struct write_commit_graph_context *ctx)
 				if (max_level > GENERATION_NUMBER_V1_MAX - 1)
 					max_level = GENERATION_NUMBER_V1_MAX - 1;
 				*topo_level_slab_at(ctx->topo_levels, current) = max_level + 1;
+			}
+		}
+	}
+	stop_progress(&ctx->progress);
+}
+
+static void compute_generation_numbers(struct write_commit_graph_context *ctx)
+{
+	int i;
+	struct commit_list *list = NULL;
+
+	if (ctx->report_progress)
+		ctx->progress = start_delayed_progress(
+					_("Computing commit graph generation numbers"),
+					ctx->commits.nr);
+	for (i = 0; i < ctx->commits.nr; i++) {
+		struct commit *c = ctx->commits.list[i];
+		timestamp_t corrected_commit_date;
+
+		repo_parse_commit(ctx->r, c);
+		corrected_commit_date = commit_graph_data_at(c)->generation;
+
+		display_progress(ctx->progress, i + 1);
+		if (corrected_commit_date != GENERATION_NUMBER_ZERO)
+			continue;
+
+		commit_list_insert(c, &list);
+		while (list) {
+			struct commit *current = list->item;
+			struct commit_list *parent;
+			int all_parents_computed = 1;
+			timestamp_t max_corrected_commit_date = 0;
+
+			for (parent = current->parents; parent; parent = parent->next) {
+				repo_parse_commit(ctx->r, parent->item);
+				corrected_commit_date = commit_graph_data_at(parent->item)->generation;
+
+				if (corrected_commit_date == GENERATION_NUMBER_ZERO) {
+					all_parents_computed = 0;
+					commit_list_insert(parent->item, &list);
+					break;
+				}
+
+				if (corrected_commit_date > max_corrected_commit_date)
+					max_corrected_commit_date = corrected_commit_date;
+			}
+
+			if (all_parents_computed) {
+				pop_commit(&list);
 
 				if (current->date && current->date > max_corrected_commit_date)
 					max_corrected_commit_date = current->date - 1;
@@ -2401,7 +2441,9 @@ int write_commit_graph(struct object_directory *odb,
 
 	validate_mixed_generation_chain(ctx->r->objects->commit_graph);
 
-	compute_generation_numbers(ctx);
+	compute_topological_levels(ctx);
+	if (ctx->write_generation_data)
+		compute_generation_numbers(ctx);
 
 	if (ctx->changed_paths)
 		compute_bloom_filters(ctx);
-- 
gitgitgadget
Previous: Junio C HamanoNext: Derrick Stolee via GitGitGadget
Message 33 of 37 in “Generation Number v2: Fix a tricky split graph bug”
  1. 0/5 Generation Number v2: Fix a tricky split graph bugDerrick Stolee via GitGitGadget, Feb 1, 2021
  2. 1/5 commit-graph: use repo_parse_commitDerrick Stolee via GitGitGadget, Feb 1, 2021
  3. Taylor BlauFeb 1, 2021
  4. 3/5 commit-graph: validate layers for generation dataDerrick Stolee via GitGitGadget, Feb 1, 2021
  5. Taylor BlauFeb 1, 2021
  6. Derrick StoleeFeb 1, 2021
  7. 4/5 commit-graph: be extra careful about mixed generationsDerrick Stolee via GitGitGadget, Feb 1, 2021
  8. Taylor BlauFeb 1, 2021
  9. Derrick StoleeFeb 1, 2021
  10. Junio C HamanoFeb 1, 2021
  11. 5/5 commit-graph: prepare commit graphDerrick Stolee via GitGitGadget, Feb 1, 2021
  12. Taylor BlauFeb 1, 2021
  13. 2/5 commit-graph: always parse before commit_graph_data_at()Derrick Stolee via GitGitGadget, Feb 1, 2021
  14. Junio C HamanoFeb 1, 2021
  15. 0/6 Generation Number v2: Fix a tricky split graph bugDerrick Stolee via GitGitGadget, Feb 2, 2021
  16. 1/6 commit-graph: use repo_parse_commitDerrick Stolee via GitGitGadget, Feb 2, 2021
  17. 3/6 commit-graph: validate layers for generation dataDerrick Stolee via GitGitGadget, Feb 2, 2021
  18. 2/6 commit-graph: always parse before commit_graph_data_at()Derrick Stolee via GitGitGadget, Feb 2, 2021
  19. Jonathan NiederFeb 3, 2021
  20. Derrick StoleeFeb 3, 2021
  21. Jonathan NiederFeb 3, 2021
  22. Derrick StoleeFeb 3, 2021
  23. Taylor BlauFeb 3, 2021
  24. Eric SunshineFeb 3, 2021
  25. Junio C HamanoFeb 3, 2021
  26. Taylor BlauFeb 3, 2021
  27. Junio C HamanoFeb 3, 2021
  28. Derrick StoleeFeb 3, 2021
  29. SZEDER GáborFeb 7, 2021
  30. Junio C HamanoFeb 7, 2021
  31. Derrick StoleeFeb 8, 2021
  32. Junio C HamanoFeb 8, 2021
  33. 4/6 commit-graph: compute generations separatelyDerrick Stolee via GitGitGadget, Feb 2, 2021
  34. 6/6 commit-graph: prepare commit graphDerrick Stolee via GitGitGadget, Feb 2, 2021
  35. 5/6 commit-graph: be extra careful about mixed generationsDerrick Stolee via GitGitGadget, Feb 2, 2021
  36. Taylor BlauFeb 2, 2021
  37. Abhishek KumarFeb 11, 2021

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.