Re: [PATCH 6/8] commit-graph.c: simplify 'fill_oids_from_commits'

From: Derrick Stolee <stolee@gmail.com>
Date: May 5, 2020, 12:05 UTC
Message-ID: <b36f9210-bc26-5ca8-b163-62d75349b381@gmail.com>
In-Reply-To: <7e9d8c1f1a124171ebb5b4d874718053d1c9064a.1588641176.git.me@ttaylorr.com>

On 5/4/2020 9:13 PM, Taylor Blau wrote:

Show 12 quoted lines

> In the previous handful of commits, both 'git commit-graph write
> --reachable' and '--stdin-commits' learned to peel tags down to the
> commits which they refer to before passing them into the commit-graph
> internals.
> 
> This makes the call to 'lookup_commit_reference_gently()' inside of
> 'fill_oids_from_commits()' a noop, since all OIDs are commits by that
> point.
> 
> As such, remove the call entirely, as well as the progress meter, which
> has been split and moved out to the callers in the aforementioned
> earlier commits.

...

>  	oidset_iter_init(commits, &iter);
>  	while ((oid = oidset_iter_next(&iter))) {

[snip removed code]

> +		ALLOC_GROW(ctx->oids.list, ctx->oids.nr + 1, ctx->oids.alloc);
> +		oidcpy(&ctx->oids.list[ctx->oids.nr], oid);
> +		ctx->oids.nr++;
>  	}

After looking at this loop, I wondered why we can't just use the oidset inside the context struct. But then I realized that the oids list expands with reachable commits and then is sorted lexicographically. Thus, this is the best time to copy from the oidset to the commit list.

I agree that dropping the progress is valuable here, since this _should_ be a very fast operation even for enormous commit sets.

Thanks, -Stolee