commit-graph: fix "Writing out commit graph" progress counter
diff mbox series

Message ID 20200709170003.3020-1-szeder.dev@gmail.com
State New
Headers show
Series
  • commit-graph: fix "Writing out commit graph" progress counter
Related show

Commit Message

SZEDER Gábor July 9, 2020, 5 p.m. UTC
76ffbca71a (commit-graph: write Bloom filters to commit graph file,
2020-04-06) added two delayed progress lines to writing the Bloom
filter index and data chunk.  This is wrong, because a single common
progress is used while writing all chunks, which is not updated while
writing these two new chunks, resulting in incomplete-looking "done"
lines:

  Expanding reachable commits in commit graph: 888679, done.
  Computing commit changed paths Bloom filters: 100% (888678/888678), done.
  Writing out commit graph in 6 passes:  66% (3554712/5332068), done.

Use the common 'struct progress' instance while writing the Bloom
filter chunks as well.

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
 commit-graph.c | 22 ++--------------------
 1 file changed, 2 insertions(+), 20 deletions(-)

Comments

Derrick Stolee July 9, 2020, 6:01 p.m. UTC | #1
On 7/9/2020 1:00 PM, SZEDER Gábor wrote:
> 76ffbca71a (commit-graph: write Bloom filters to commit graph file,
> 2020-04-06) added two delayed progress lines to writing the Bloom
> filter index and data chunk.  This is wrong, because a single common
> progress is used while writing all chunks, which is not updated while
> writing these two new chunks, resulting in incomplete-looking "done"
> lines:
> 
>   Expanding reachable commits in commit graph: 888679, done.
>   Computing commit changed paths Bloom filters: 100% (888678/888678), done.
>   Writing out commit graph in 6 passes:  66% (3554712/5332068), done.
> 
> Use the common 'struct progress' instance while writing the Bloom
> filter chunks as well.

Thanks for finding this. It's a clearly correct way to go,
and is one of the things that did not get updated properly
between the old prototype when applying it on the new code
that included this ctx->progress pattern.

Junio: head's up that this will conflict with the final patch
in ds/maintenance. I'll remove my edits to these methods in
my v2 to make that merge a bit easier.

Thanks,
-Stolee
Derrick Stolee July 9, 2020, 6:20 p.m. UTC | #2
On 7/9/2020 2:01 PM, Derrick Stolee wrote:
> Junio: head's up that this will conflict with the final patch
> in ds/maintenance. I'll remove my edits to these methods in
> my v2 to make that merge a bit easier.
Or, I'm getting confused, because I changed start_progress()
calls in midx.c, not commit-graph.c. Please ignore my scattered
brain.

Thanks,
-Stolee

Patch
diff mbox series

diff --git a/commit-graph.c b/commit-graph.c
index aaf3327ede..65cf32637c 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -1086,23 +1086,14 @@  static void write_graph_chunk_bloom_indexes(struct hashfile *f,
 	struct commit **list = ctx->commits.list;
 	struct commit **last = ctx->commits.list + ctx->commits.nr;
 	uint32_t cur_pos = 0;
-	struct progress *progress = NULL;
-	int i = 0;
-
-	if (ctx->report_progress)
-		progress = start_delayed_progress(
-			_("Writing changed paths Bloom filters index"),
-			ctx->commits.nr);
 
 	while (list < last) {
 		struct bloom_filter *filter = get_bloom_filter(ctx->r, *list, 0);
 		cur_pos += filter->len;
-		display_progress(progress, ++i);
+		display_progress(ctx->progress, ++ctx->progress_cnt);
 		hashwrite_be32(f, cur_pos);
 		list++;
 	}
-
-	stop_progress(&progress);
 }
 
 static void write_graph_chunk_bloom_data(struct hashfile *f,
@@ -1111,13 +1102,6 @@  static void write_graph_chunk_bloom_data(struct hashfile *f,
 {
 	struct commit **list = ctx->commits.list;
 	struct commit **last = ctx->commits.list + ctx->commits.nr;
-	struct progress *progress = NULL;
-	int i = 0;
-
-	if (ctx->report_progress)
-		progress = start_delayed_progress(
-			_("Writing changed paths Bloom filters data"),
-			ctx->commits.nr);
 
 	hashwrite_be32(f, settings->hash_version);
 	hashwrite_be32(f, settings->num_hashes);
@@ -1125,12 +1109,10 @@  static void write_graph_chunk_bloom_data(struct hashfile *f,
 
 	while (list < last) {
 		struct bloom_filter *filter = get_bloom_filter(ctx->r, *list, 0);
-		display_progress(progress, ++i);
+		display_progress(ctx->progress, ++ctx->progress_cnt);
 		hashwrite(f, filter->data, filter->len * sizeof(unsigned char));
 		list++;
 	}
-
-	stop_progress(&progress);
 }
 
 static int oid_compare(const void *_a, const void *_b)