Re: [PATCH v3 3/8] commit-graph: combine generation computations
- From
- Jonathan Tan <jonathantanmy@google.com>
- Date
- Mar 15, 2023, 22:49 UTC
- Message-ID
- <20230315224958.169443-1-jonathantanmy@google.com>
- In-Reply-To
- <3b15e9df770a118331a1b25f51de8ce97c1b7cab.1678902343.git.gitgitgadget@gmail.com>
"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 30 quoted lines
> +static void compute_reachable_generation_numbers_1(
> + struct compute_generation_info *info,
> + int generation_version)
> {
> int i;
> struct commit_list *list = NULL;
>
> - if (ctx->report_progress)
> - ctx->progress = start_delayed_progress(
> - _("Computing commit graph topological levels"),
> - ctx->commits.nr);
> - for (i = 0; i < ctx->commits.nr; i++) {
> - struct commit *c = ctx->commits.list[i];
> - uint32_t level;
> + for (i = 0; i < info->commits->nr; i++) {
> + struct commit *c = info->commits->list[i];
> + timestamp_t gen;
> + repo_parse_commit(info->r, c);
> + gen = info->get_generation(c, info->data);
>
> - repo_parse_commit(ctx->r, c);
> - level = *topo_level_slab_at(ctx->topo_levels, c);
> + display_progress(info->progress, info->progress_cnt + 1);
>
> - display_progress(ctx->progress, i + 1);
> - if (level != GENERATION_NUMBER_ZERO)
> + if (gen != GENERATION_NUMBER_ZERO && gen != GENERATION_NUMBER_INFINITY)
> continue;
>
> commit_list_insert(c, &list);So this replaces a call to display_progress with another...
Show 11 quoted lines
> if (all_parents_computed) {
> pop_commit(&list);
> -
> - if (max_level > GENERATION_NUMBER_V1_MAX - 1)
> - max_level = GENERATION_NUMBER_V1_MAX - 1;
> - *topo_level_slab_at(ctx->topo_levels, current) = max_level + 1;
> + gen = compute_generation_from_max(
> + current, max_gen,
> + generation_version);
> + info->set_generation(current, gen, info->data);
> }...here is where set_generation is called...
Show 6 quoted lines
> +static void set_topo_level(struct commit *c, timestamp_t t, void *data)
> +{
> + struct write_commit_graph_context *ctx = data;
> + *topo_level_slab_at(ctx->topo_levels, c) = (uint32_t)t;
> + display_progress(ctx->progress, ctx->progress_cnt + 1);
> +}...is this display_progress() redundant? (set_topo_level() is one of the possibilities that set_generation could be assigned to.) There already seems to be one at the top. Further supporting my query is the fact that in the hunk containing set_generation, there is no progress report on the LHS of the diff.
Show 7 quoted lines
> +static void set_generation_v2(struct commit *c, timestamp_t t, void *data)
> +{
> + struct write_commit_graph_context *ctx = data;
> + struct commit_graph_data *g = commit_graph_data_at(c);
> + g->generation = (uint32_t)t;
> + display_progress(ctx->progress, ctx->progress_cnt + 1);
> +}Likewise for this function.
Everything else up to and including this patch looks good.