Re: [PATCH v3 01/11] commit-graph: fix regression when computing bloom filter
From: Jakub Narębski <hidden>
Date: 2020-08-17 22:30:06
"Abhishek Kumar via GitGitGadget" [off-list ref] writes:
Subject: [PATCH v3 01/11] commit-graph: fix regression when computing bloom filter
s/bloom filter/Bloom filters/
From: Abhishek Kumar <redacted> commit_gen_cmp is used when writing a commit-graph to sort commits in generation order before computing Bloom filters. Since c49c82aa (commit: move members graph_pos, generation to a slab, 2020-06-17) made it so that 'commit_graph_generation()' returns 'GENERATION_NUMBER_INFINITY' during writing, we cannot call it within this function. Instead, access the generation number directly through the slab (i.e., by calling 'commit_graph_data_at(c)->generation') in order to access it while writing.
Two things that might not be obvious from the commit message: - Is commit_gen_cmp in commit-graph.c used by anything but writing Bloom filters for changed paths? - That the generation number is computed during `commit-graph write` before computing Bloom filters. Also, after this series 'generation' would be generation number v2, that is corrected commit date, and not v1, that is topological levels. We should check, just in case, that it does not lead to significant performance regression for `git commit-graph write --reachable <...>` case (the one that uses commit_gen_cmp sort).
quoted hunk
Signed-off-by: Abhishek Kumar <redacted> --- commit-graph.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)diff --git a/commit-graph.c b/commit-graph.c index e51c91dd5b..ace7400a1a 100644 --- a/commit-graph.c +++ b/commit-graph.c@@ -144,8 +144,8 @@ static int commit_gen_cmp(const void *va, const void *vb) const struct commit *a = *(const struct commit **)va; const struct commit *b = *(const struct commit **)vb; - uint32_t generation_a = commit_graph_generation(a); - uint32_t generation_b = commit_graph_generation(b); + uint32_t generation_a = commit_graph_data_at(a)->generation; + uint32_t generation_b = commit_graph_data_at(b)->generation;
Nice and easy.
/* lower generation commits first */ if (generation_a < generation_b) return -1;
Best, -- Jakub Narębski