From: Taylor Blau <me@ttaylorr.com>
Date: Thu, 11 Sep 2025 22:16:08 GMT
Subject: Re: [PATCH 2/6] revision: drop explicit check for commit graph
Message-ID: <aMNKKOo+BsNxGKgh@nand.local>
In-Reply-To: <20250904-b4-pks-commit-graph-via-source-v1-2-d932c2481e1a@pks.im>

On Thu, Sep 04, 2025 at 02:49:56PM +0200, Patrick Steinhardt wrote:
> When filtering down revisions by paths we know to use bloom filters from
> the commit graph, if we have any. The entry point for this is in
> `check_maybe_different_in_bloom_filter()`, where we first verify that:
>
>   - We do have a commit graph.
>
>   - That the commit is contained therein by checking that we have a
>     proper generation number.
>
>   - And that the graph contains a bloom filter.
>
> The first check is somewhat redundant though: if we don't have a commit
> graph, then the second check would already tell us that we don't have a
> generation number for the specific commit.

Makes sense; and we already inspect the commit_graph_data_slab
regardless of whether or not we know we have a commit-graph, so this
clean-up makes sense to me, too.

> In theory this could be seen as a performance optimization to
> short-circuit for scenarios where there is no commit graph. But in
> practice this shouldn't matter: if there is no commit graph, then the
> commit graph data slab would also be unpopulated and thus a lookup of
> the commit should happen in constant time.

Yeah, this should be a few extra cycles at most. I think it should be
fine.

Taylor