Re: [RFC PATCH] Not computing changed path filter for root commits

From: Taylor Blau <me@ttaylorr.com>
Date: Sep 19, 2023, 18:21 UTC
Message-ID: <ZQnmwID4PNlB5ME0@nand.local>
In-Reply-To: <20230911223157.446269-1-jonathantanmy@google.com>

On Mon, Sep 11, 2023 at 03:31:56PM -0700, Jonathan Tan wrote:

Show 9 quoted lines

> SZEDER Gábor suggested [2] that we change the revision walk to read
> changed path filters also for root commits, but I don't think that's
> possible - we have to tie reading changed path filters to when we read
> trees, and right now, we don't seem to read trees when evaluating root
> commits (rev_compare_tree() in revision.c is in the only code path that
> uses changed path filters, and it itself is only called per-parent and
> thus not called for root commits). The alternative is to not generate
> changed path filters for root commits (or what I did in this patch,
> which is to generate an all-1 filter), which seems reasonable to me.

I think between the two, the all-1's filter is the more sensible choice, since not computing a filter is typically reserved for blowing past the `commitGraph.maxNewFilters` setting.

But, I agree with Gábor down-thread that we could instead teach `rev_same_tree_as_empty()` to be aware of Bloom filters, which I think would accomplish our goal of reading Bloom filters at the root commit while not having to tweak their generation.

Thanks, Taylor