Thread (1 message) 1 message, 1 author, 2023-09-15

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

From: Junio C Hamano <hidden>
Date: 2023-09-15 18:25:57

Jonathan Tan [off-list ref] writes:
This is following the discussion about adding a new changed path filter
version due to the current implementation of murmur3 not matching the
algorithm. [1]

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 know this is a very silly question, but if the filter is not read
for root commits at runtime, does it matter if a filter is created
for them beforehand (or not)?  They will not be read whether if they
exist or not, no?  One observation in the thread [2] appears in was:

    In several of the above test cases test_bloom_filters_used is invoked
    in a repository with only a root commit, so they don't check that
    the output is the same with and without Bloom filters.

i.e. the check would be ineffective with the current system that we
know does not use the filter for a root commit even if it existed.
But would it be an improvement to add a filter to a root commit and
test with the filter enabled and disabled to compare the results, if
we know the filter is not used anyway?
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help