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

[PATCH 11/34] commit-graph: allocate the 'struct chunk_info' array dinamically

From
SZEDER Gábor <szeder.dev@gmail.com>
Date
May 29, 2020, 08:50 UTC
Message-ID
<20200529085038.26008-12-szeder.dev@gmail.com>
In-Reply-To
<20200529085038.26008-1-szeder.dev@gmail.com>

During writing a new commit-graph file, the size of the 'struct chunk_info' array is hardcoded to be the maximum number of possible chunks plus one for the terminating label. This array size must be adjusted each time when a new chunk is added, which can be easily forgotten.

So let's make the constant-sized 'struct chunk_info' array an ALLOC_GROW()-able array instead.

There are a couple of cases where writing a new commit-graph file returns with error; those spots were adjusted to free the dinamically allocated 'struct chunk_info' array.

Arguably the several ALLOC_GROW() call sites look unusal, because we usually call ALLOC_GROW() in loops, once in each iteration... However, this way anyone who adds a new chunk will notice that they have to cargo-cult the ALLOC_GROW() call as well.

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
 commit-graph.c | 74 ++++++++++++++++++++++++++++++++------------------
 1 file changed, 47 insertions(+), 27 deletions(-)
diff --git a/commit-graph.c b/commit-graph.c
index d95c739c10..c8ba17e457 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -1355,12 +1355,13 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx)
 	int fd;
 	struct hashfile *f;
 	struct lock_file lk = LOCK_INIT;
-	struct chunk_info chunks[6];
+	struct chunk_info *chunks = NULL;
+	int chunks_nr = 0, chunks_alloc = 0;
 	const unsigned hashsz = the_hash_algo->rawsz;
 	struct strbuf progress_title = STRBUF_INIT;
-	int num_chunks = 3;
 	uint64_t chunk_offset;
 	struct object_id file_hash;
+	int ret = 0;
 
 	if (ctx->split) {
 		struct strbuf tmp_file = STRBUF_INIT;
@@ -1398,35 +1399,48 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx)
 		f = hashfd(lk.tempfile->fd, lk.tempfile->filename.buf);
 	}
 
-	chunks[0].id = GRAPH_CHUNKID_OIDFANOUT;
-	chunks[0].size = GRAPH_FANOUT_SIZE;
-	chunks[1].id = GRAPH_CHUNKID_OIDLOOKUP;
-	chunks[1].size = hashsz * ctx->commits.nr;
-	chunks[2].id = GRAPH_CHUNKID_DATA;
-	chunks[2].size = (hashsz + 16) * ctx->commits.nr;
+	ALLOC_GROW(chunks, chunks_nr + 1, chunks_alloc);
+	chunks[chunks_nr].id = GRAPH_CHUNKID_OIDFANOUT;
+	chunks[chunks_nr].size = GRAPH_FANOUT_SIZE;
+	chunks_nr++;
+	ALLOC_GROW(chunks, chunks_nr + 1, chunks_alloc);
+	chunks[chunks_nr].id = GRAPH_CHUNKID_OIDLOOKUP;
+	chunks[chunks_nr].size = hashsz * ctx->commits.nr;
+	chunks_nr++;
+	ALLOC_GROW(chunks, chunks_nr + 1, chunks_alloc);
+	chunks[chunks_nr].id = GRAPH_CHUNKID_DATA;
+	chunks[chunks_nr].size = (hashsz + 16) * ctx->commits.nr;
+	chunks_nr++;
 	if (ctx->num_extra_edges) {
-		chunks[num_chunks].id = GRAPH_CHUNKID_EXTRAEDGES;
-		chunks[num_chunks].size = 4 * ctx->num_extra_edges;
-		num_chunks++;
+		ALLOC_GROW(chunks, chunks_nr + 1, chunks_alloc);
+		chunks[chunks_nr].id = GRAPH_CHUNKID_EXTRAEDGES;
+		chunks[chunks_nr].size = 4 * ctx->num_extra_edges;
+		chunks_nr++;
 	}
 	if (ctx->num_commit_graphs_after > 1) {
-		chunks[num_chunks].id = GRAPH_CHUNKID_BASE;
-		chunks[num_chunks].size = hashsz * (ctx->num_commit_graphs_after - 1);
-		num_chunks++;
+		ALLOC_GROW(chunks, chunks_nr + 1, chunks_alloc);
+		chunks[chunks_nr].id = GRAPH_CHUNKID_BASE;
+		chunks[chunks_nr].size = hashsz * (ctx->num_commit_graphs_after - 1);
+		chunks_nr++;
 	}
 
-	chunks[num_chunks].id = 0;
-	chunks[num_chunks].size = 0;
+	ALLOC_GROW(chunks, chunks_nr + 1, chunks_alloc);
+	chunks[chunks_nr].id = 0;
+	chunks[chunks_nr].size = 0;
+	/*
+	 * Do not increase 'chunks_nr', so it still reflects the number of
+	 * actual chunks, without the Chunk Lookup table's terminating label.
+	 */
 
 	hashwrite_be32(f, GRAPH_SIGNATURE);
 
 	hashwrite_u8(f, GRAPH_VERSION);
 	hashwrite_u8(f, oid_version());
-	hashwrite_u8(f, num_chunks);
+	hashwrite_u8(f, chunks_nr);
 	hashwrite_u8(f, ctx->num_commit_graphs_after - 1);
 
-	chunk_offset = 8 + (num_chunks + 1) * GRAPH_CHUNKLOOKUP_WIDTH;
-	for (i = 0; i <= num_chunks; i++) {
+	chunk_offset = 8 + (chunks_nr + 1) * GRAPH_CHUNKLOOKUP_WIDTH;
+	for (i = 0; i <= chunks_nr; i++) {
 		uint32_t chunk_write[3];
 
 		chunk_write[0] = htonl(chunks[i].id);
@@ -1441,11 +1455,11 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx)
 		strbuf_addf(&progress_title,
 			    Q_("Writing out commit graph in %d pass",
 			       "Writing out commit graph in %d passes",
-			       num_chunks),
-			    num_chunks);
+			       chunks_nr),
+			    chunks_nr);
 		ctx->progress = start_delayed_progress(
 			progress_title.buf,
-			num_chunks * ctx->commits.nr);
+			chunks_nr * ctx->commits.nr);
 	}
 	write_graph_chunk_fanout(f, ctx);
 	write_graph_chunk_oids(f, hashsz, ctx);
@@ -1454,7 +1468,8 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx)
 		write_graph_chunk_extra_edges(f, ctx);
 	if (ctx->num_commit_graphs_after > 1 &&
 	    write_graph_chunk_base(f, ctx)) {
-		return -1;
+		ret = -1;
+		goto cleanup;
 	}
 	stop_progress(&ctx->progress);
 	strbuf_release(&progress_title);
@@ -1481,7 +1496,8 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx)
 
 		if (!chainf) {
 			error(_("unable to open commit-graph chain file"));
-			return -1;
+			ret = -1;
+			goto cleanup;
 		}
 
 		if (ctx->base_graph_name) {
@@ -1493,7 +1509,8 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx)
 
 				if (result) {
 					error(_("failed to rename base commit-graph file"));
-					return -1;
+					ret = -1;
+					goto cleanup;
 				}
 			}
 		} else {
@@ -1513,13 +1530,16 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx)
 
 		if (result) {
 			error(_("failed to rename temporary commit-graph file"));
-			return -1;
+			ret = -1;
+			goto cleanup;
 		}
 	}
 
 	commit_lock_file(&lk);
 
-	return 0;
+cleanup:
+	free(chunks);
+	return ret;
 }
 
 static void split_graph_merge_strategy(struct write_commit_graph_context *ctx)

-- 
2.27.0.rc1.431.g5c813f95dc
Previous: SZEDER GáborNext: SZEDER Gábor
Message 15 of 43 in “An alternative modified path Bloom filters implementation”
  1. 00/34 An alternative modified path Bloom filters implementationSZEDER Gábor, May 29, 2020
  2. 01/34 tree-walk.c: don't match submodule entries for 'submod/anything'SZEDER Gábor, May 29, 2020
  3. 02/34 commit-graph: fix parsing the Chunk Lookup tableSZEDER Gábor, May 29, 2020
  4. 03/34 commit-graph-format.txt: all multi-byte numbers are in network byte orderSZEDER Gábor, May 29, 2020
  5. 04/34 commit-slab: add a function to deep free entries on the slabSZEDER Gábor, May 29, 2020
  6. Derrick StoleeJun 4, 2020
  7. SZEDER GáborJun 27, 2020
  8. Derrick StoleeJun 29, 2020
  9. 05/34 diff.h: drop diff_tree_oid() & friends' return valueSZEDER Gábor, May 29, 2020
  10. 07/34 commit-graph: simplify parse_commit_graph() #1SZEDER Gábor, May 29, 2020
  11. 06/34 commit-graph: clean up #includesSZEDER Gábor, May 29, 2020
  12. 09/34 commit-graph: simplify write_commit_graph_file() #1SZEDER Gábor, May 29, 2020
  13. 10/34 commit-graph: simplify write_commit_graph_file() #2SZEDER Gábor, May 29, 2020
  14. 08/34 commit-graph: simplify parse_commit_graph() #2SZEDER Gábor, May 29, 2020
  15. 11/34 commit-graph: allocate the 'struct chunk_info' array dinamicallySZEDER Gábor, May 29, 2020
  16. 12/34 commit-graph: unify the signatures of all write_graph_chunk_*() functionsSZEDER Gábor, May 29, 2020
  17. 13/34 commit-graph: simplify write_commit_graph_file() #3SZEDER Gábor, May 29, 2020
  18. 14/34 commit-graph: check chunk sizes after writingSZEDER Gábor, May 29, 2020
  19. 16/34 Add a generic and minimal Bloom filter implementationSZEDER Gábor, May 29, 2020
  20. 17/34 Import a streaming-capable Murmur3 hash function implementationSZEDER Gábor, May 29, 2020
  21. 15/34 commit-graph-format.txt: document the modified path Bloom filter chunksSZEDER Gábor, May 29, 2020
  22. SZEDER GáborJun 2, 2020
  23. 18/34 commit-graph: write "empty" Modified Path Bloom Filter Index chunkSZEDER Gábor, May 29, 2020
  24. 19/34 commit-graph: add commit slab for modified path Bloom filtersSZEDER Gábor, May 29, 2020
  25. 20/34 commit-graph: fill the Modified Path Bloom Filter Index chunkSZEDER Gábor, May 29, 2020
  26. 21/34 commit-graph: load and use the Modified Path Bloom Filter Index chunkSZEDER Gábor, May 29, 2020
  27. 27/34 commit-graph: load modified path Bloom filters for merge commitsSZEDER Gábor, May 29, 2020
  28. 28/34 commit-graph: write Modified Path Bloom Filter Merge Index chunkSZEDER Gábor, May 29, 2020
  29. 30/34 commit-graph: move write_commit_graph_reachable below write_commit_graphSZEDER Gábor, May 29, 2020
  30. 29/34 commit-graph: extract init and free write_commit_graph_contextSZEDER Gábor, May 29, 2020
  31. 31/34 t7007-show: make the first test compatible with the next patchSZEDER Gábor, May 29, 2020
  32. 26/34 commit-graph: deduplicate modified path Bloom filtersSZEDER Gábor, May 29, 2020
  33. 25/34 commit-graph: check embedded modified path Bloom filters with a maskSZEDER Gábor, May 29, 2020
  34. 24/34 commit-graph: check all leading directories in modified path Bloom filtersSZEDER Gábor, May 29, 2020
  35. 23/34 commit-graph: load and use the Modified Path Bloom Filters chunkSZEDER Gábor, May 29, 2020
  36. 22/34 commit-graph: write the Modified Path Bloom Filters chunkSZEDER Gábor, May 29, 2020
  37. 34/34 commit-graph: use modified path Bloom filters with wildcards, if possibleSZEDER Gábor, May 29, 2020
  38. 33/34 commit-graph: write modified path Bloom filters in "history order"SZEDER Gábor, May 29, 2020
  39. 32/34 PoC commit-graph: use revision walk machinery for '--reachable'SZEDER Gábor, May 29, 2020
  40. Derrick StoleeMay 29, 2020
  41. Johannes SchindelinMay 28, 2020
  42. Taylor BlauJun 1, 2020
  43. Junio C HamanoJun 2, 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.