Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes:
> From: SZEDER Gábor <szeder.dev@xxxxxxxxx>
>
> 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.
> 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?
