Re: [PATCH 3/3] entry: show finer-grained counter in "Filtering content" progress line
From: Junio C Hamano <hidden>
Date: 2021-07-23 22:01:52
Ævar Arnfjörð Bjarmason [off-list ref] writes:
From: SZEDER Gábor <redacted>
The "Filtering content" progress in entry.c:finish_delayed_checkout()
is unusual because of how it calculates the progress count and because
it shows the progress of a nested loop. It works basically like this:
start_delayed_progress(p, nr_of_paths_to_filter)
for_each_filter {
display_progress(p, nr_of_paths_to_filter - nr_of_paths_still_left_to_filter)
for_each_path_handled_by_the_current_filter {
checkout_entry()
}
}
stop_progress(p)
There are two issues with this approach:
- The work done by the last filter (or the only filter if there is
only one) is never counted, so if the last filter still has some
paths to process, then the counter shown in the "done" progress
line will not match the expected total.
This would cause a BUG() in an upcoming change that adds an
assertion checking if the "total" at the end matches the last
progress bar update..So the other series will semantically depend on this 3-patch series? Just making sure that is the intended topic structure.
quoted hunk
diff --git a/entry.c b/entry.c index 125fabdbd5..d92dd020b3 100644 --- a/entry.c +++ b/entry.c@@ -162,7 +162,7 @@ static int remove_available_paths(struct string_list_item *item, void *cb_data) int finish_delayed_checkout(struct checkout *state, int *nr_checkouts) { int errs = 0; - unsigned delayed_object_count; + unsigned processed_paths = 0; off_t filtered_bytes = 0; struct string_list_item *filter, *path; struct progress *progress;@@ -172,12 +172,10 @@ int finish_delayed_checkout(struct checkout *state, int *nr_checkouts) return errs; dco->state = CE_RETRY; - delayed_object_count = dco->paths.nr; - progress = start_delayed_progress(_("Filtering content"), delayed_object_count); + progress = start_delayed_progress(_("Filtering content"), dco->paths.nr); while (dco->filters.nr > 0) { for_each_string_list_item(filter, &dco->filters) { struct string_list available_paths = STRING_LIST_INIT_NODUP; - display_progress(progress, delayed_object_count - dco->paths.nr); if (!async_query_available_blobs(filter->string, &available_paths)) { /* Filter reported an error */@@ -224,6 +222,7 @@ int finish_delayed_checkout(struct checkout *state, int *nr_checkouts) ce = index_file_exists(state->istate, path->string, strlen(path->string), 0); if (ce) { + display_progress(progress, ++processed_paths); errs |= checkout_entry(ce, state, NULL, nr_checkouts); filtered_bytes += ce->ce_stat_data.sd_size; display_throughput(progress, filtered_bytes);
Hmph. A missing cache entries will not increment processed; would that cause stop_progress() to see at the end the counter that is smaller than dco->paths.nr we saw at the beginning?