Re: [PATCH] commit-graph: retain commit slab when closing NULL commit_graph

From

Junio C Hamano <gitster@pobox.com>

Date

Jan 10, 2024, 16:38 UTC

Message-ID

<xmqq34v5dtz9.fsf@gitster.g>

In-Reply-To

<20240110113914.GE16674@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> I think a simpler solution would be that upon clearing the slab, we
> either "finish" each commit (filling in the maybe_tree field) or
> "unparse" it (unsetting the parsed flag, and then doing a regular and/or
> graph lookup if it is accessed again later).

Wow, that's clever.

> It should be easy-ish to iterate through the slab and look at the
> commits that are mentioned in it. Though maybe not? Each commit knows
> its slab-id, but I'm not sure if we have a master list of commits to go
> the other way.

We have table of all in-core objects, don't we?

> +	 * This will throw away the parents list, which is potentially sketchy.
> +	 * A better version of this would just unset commit->object.parsed
> +	 * and then do a minimal version of parse_commit() that _just_ loads
> +	 * the tree data (and/or graph position if available).

> +	 *
> +	 * Naturally we'd need to drop the "const" from our commit above, too.
> +	 */
> +	unparse_commit(r, &commit->object.oid);
> +	repo_parse_commit(r, commit);
> +
>  	return NULL;
>  }
>  
> I dunno. I do feel like this is a lurking maintenance headache, and
> might even be a triggerable bug. But without knowing of a way that it
> happens in the current code base, it feels like it would be easy to make
> things worse rather than better.

Unfortunately I share the feeling X-<.

Thanks.