| Message ID | adc03eee4ac8a0911bfd2a7ae03364ef0e744ef0.1587068044.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Integrate changed-path Bloom filters with 'git blame' | expand |
On Thu, Apr 16, 2020 at 08:14:02PM +0000, Derrick Stolee via GitGitGadget wrote: > From: Derrick Stolee <dstolee@microsoft.com> > > The changed-path Bloom filters work only when we can compute an > explicit Bloom filter key in advance. When a pathspec is given > that allows case-insensitive checks or wildcard matching, we > must disable the Bloom filter performance checks. > > By checking the pathspec in prepare_to_use_bloom_filters(), we > avoid setting up the Bloom filter data and thus revert to the > usual logic. > > Before this change, the following tests would fail*: > > t6004-rev-list-path-optim.sh (Tests 6-7) > t6130-pathspec-noglob.sh (Tests 3-6) > t6131-pathspec-icase.sh (Tests 3-5) > > *These tests would fail when using GIT_TEST_COMMIT_GRAPH and > GIT_TEST_COMMIT_GRAPH_BLOOM_FILTERS except that the latter > environment variable was not set up correctly to write the changed- > path Bloom filters in the test suite. That will be fixed in the > next change. > > Helped-by: Taylor Blau <me@ttaylorr.com> > Signed-off-by: Derrick Stolee <dstolee@microsoft.com> > --- > revision.c | 19 ++++++++++++++++++- > 1 file changed, 18 insertions(+), 1 deletion(-) > > diff --git a/revision.c b/revision.c > index 2b06ee739c8..f78c636e4d0 100644 > --- a/revision.c > +++ b/revision.c > @@ -650,6 +650,20 @@ static void trace2_bloom_filter_statistics_atexit(void) > jw_release(&jw); > } > > +static int forbid_bloom_filters(struct pathspec *spec) > +{ > + if (spec->has_wildcard) > + return 1; > + if (spec->nr > 1) > + return 1; > + if (spec->magic & ~PATHSPEC_LITERAL) Nit: spec->magic is the bitwise OR combination of all spec->items[i].magic, so checking the latter below is unnecessary. > + return 1; > + if (spec->nr && (spec->items[0].magic & ~PATHSPEC_LITERAL)) > + return 1; > + > + return 0; > +} > + > static void prepare_to_use_bloom_filter(struct rev_info *revs) > { > struct pathspec_item *pi; > @@ -659,7 +673,10 @@ static void prepare_to_use_bloom_filter(struct rev_info *revs) > int len; > > if (!revs->commits) > - return; > + return; > + > + if (forbid_bloom_filters(&revs->prune_data)) > + return; > > repo_parse_commit(revs->repo, revs->commits->item); > > -- > gitgitgadget >
diff --git a/revision.c b/revision.c index 2b06ee739c8..f78c636e4d0 100644 --- a/revision.c +++ b/revision.c @@ -650,6 +650,20 @@ static void trace2_bloom_filter_statistics_atexit(void) jw_release(&jw); } +static int forbid_bloom_filters(struct pathspec *spec) +{ + if (spec->has_wildcard) + return 1; + if (spec->nr > 1) + return 1; + if (spec->magic & ~PATHSPEC_LITERAL) + return 1; + if (spec->nr && (spec->items[0].magic & ~PATHSPEC_LITERAL)) + return 1; + + return 0; +} + static void prepare_to_use_bloom_filter(struct rev_info *revs) { struct pathspec_item *pi; @@ -659,7 +673,10 @@ static void prepare_to_use_bloom_filter(struct rev_info *revs) int len; if (!revs->commits) - return; + return; + + if (forbid_bloom_filters(&revs->prune_data)) + return; repo_parse_commit(revs->repo, revs->commits->item);