Skip to content

Commit 2a6ce09

Browse files
brandb97gitster
authored andcommitted
bloom: optimize multiple pathspec items in revision
To enable optimize multiple pathspec items in revision traversal, return 0 if all pathspec item is literal in forbid_bloom_filters(). Add for loops to initialize and check each pathspec item's bloom_keyvec when optimization is possible. Add new test cases in t/t4216-log-bloom.sh to ensure - consistent results between the optimization for multiple pathspec items using bloom filter and the case without bloom filter optimization. - does not use bloom filter if any pathspec item is not literal. With these optimizations, we get some improvements for multi-pathspec runs of 'git log'. First, in the Git repository we see these modest results: Benchmark 1: old Time (mean ± σ): 73.1 ms ± 2.9 ms Range (min … max): 69.9 ms … 84.5 ms 42 runs Benchmark 2: new Time (mean ± σ): 55.1 ms ± 2.9 ms Range (min … max): 51.1 ms … 61.2 ms 52 runs Summary 'new' ran 1.33 ± 0.09 times faster than 'old' But in a larger repo, such as the LLVM project repo below, we get even better results: Benchmark 1: old Time (mean ± σ): 1.974 s ± 0.006 s Range (min … max): 1.960 s … 1.983 s 10 runs Benchmark 2: new Time (mean ± σ): 262.9 ms ± 2.4 ms Range (min … max): 257.7 ms … 266.2 ms 11 runs Summary 'new' ran 7.51 ± 0.07 times faster than 'old' Signed-off-by: Derrick Stolee <[email protected]> [ly: rename convert_pathspec_to_filter() to convert_pathspec_to_bloom_keyvec()] Signed-off-by: Lidong Yan <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 937153d commit 2a6ce09

File tree

2 files changed

+25
-19
lines changed

2 files changed

+25
-19
lines changed

revision.c

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -675,12 +675,11 @@ static int forbid_bloom_filters(struct pathspec *spec)
675675
{
676676
if (spec->has_wildcard)
677677
return 1;
678-
if (spec->nr > 1)
679-
return 1;
680678
if (spec->magic & ~PATHSPEC_LITERAL)
681679
return 1;
682-
if (spec->nr && (spec->items[0].magic & ~PATHSPEC_LITERAL))
683-
return 1;
680+
for (size_t nr = 0; nr < spec->nr; nr++)
681+
if (spec->items[nr].magic & ~PATHSPEC_LITERAL)
682+
return 1;
684683

685684
return 0;
686685
}
@@ -733,13 +732,15 @@ static void prepare_to_use_bloom_filter(struct rev_info *revs)
733732
if (!revs->pruning.pathspec.nr)
734733
return;
735734

736-
revs->bloom_keyvecs_nr = 1;
737-
CALLOC_ARRAY(revs->bloom_keyvecs, 1);
735+
revs->bloom_keyvecs_nr = revs->pruning.pathspec.nr;
736+
CALLOC_ARRAY(revs->bloom_keyvecs, revs->bloom_keyvecs_nr);
738737

739-
if (convert_pathspec_to_bloom_keyvec(&revs->bloom_keyvecs[0],
740-
&revs->pruning.pathspec.items[0],
741-
revs->bloom_filter_settings))
742-
goto fail;
738+
for (int i = 0; i < revs->pruning.pathspec.nr; i++) {
739+
if (convert_pathspec_to_bloom_keyvec(&revs->bloom_keyvecs[i],
740+
&revs->pruning.pathspec.items[i],
741+
revs->bloom_filter_settings))
742+
goto fail;
743+
}
743744

744745
if (trace2_is_enabled() && !bloom_filter_atexit_registered) {
745746
atexit(trace2_bloom_filter_statistics_atexit);

t/t4216-log-bloom.sh

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,9 @@ sane_unset GIT_TRACE2_CONFIG_PARAMS
6666

6767
setup () {
6868
rm -f "$TRASH_DIRECTORY/trace.perf" &&
69-
git -c core.commitGraph=false log --pretty="format:%s" $1 >log_wo_bloom &&
70-
GIT_TRACE2_PERF="$TRASH_DIRECTORY/trace.perf" git -c core.commitGraph=true log --pretty="format:%s" $1 >log_w_bloom
69+
eval git -c core.commitGraph=false log --pretty="format:%s" "$1" >log_wo_bloom &&
70+
eval "GIT_TRACE2_PERF=\"$TRASH_DIRECTORY/trace.perf\"" \
71+
git -c core.commitGraph=true log --pretty="format:%s" "$1" >log_w_bloom
7172
}
7273

7374
test_bloom_filters_used () {
@@ -138,10 +139,6 @@ test_expect_success 'git log with --walk-reflogs does not use Bloom filters' '
138139
test_bloom_filters_not_used "--walk-reflogs -- A"
139140
'
140141

141-
test_expect_success 'git log -- multiple path specs does not use Bloom filters' '
142-
test_bloom_filters_not_used "-- file4 A/file1"
143-
'
144-
145142
test_expect_success 'git log -- "." pathspec at root does not use Bloom filters' '
146143
test_bloom_filters_not_used "-- ."
147144
'
@@ -151,9 +148,17 @@ test_expect_success 'git log with wildcard that resolves to a single path uses B
151148
test_bloom_filters_used "-- *renamed"
152149
'
153150

154-
test_expect_success 'git log with wildcard that resolves to a multiple paths does not uses Bloom filters' '
155-
test_bloom_filters_not_used "-- *" &&
156-
test_bloom_filters_not_used "-- file*"
151+
test_expect_success 'git log with multiple literal paths uses Bloom filter' '
152+
test_bloom_filters_used "-- file4 A/file1" &&
153+
test_bloom_filters_used "-- *" &&
154+
test_bloom_filters_used "-- file*"
155+
'
156+
157+
test_expect_success 'git log with path contains a wildcard does not use Bloom filter' '
158+
test_bloom_filters_not_used "-- file\*" &&
159+
test_bloom_filters_not_used "-- A/\* file4" &&
160+
test_bloom_filters_not_used "-- file4 A/\*" &&
161+
test_bloom_filters_not_used "-- * A/\*"
157162
'
158163

159164
test_expect_success 'setup - add commit-graph to the chain without Bloom filters' '

0 commit comments

Comments
 (0)