Skip to content

Commit 7cae762

Browse files
ffyuandagitster
authored andcommitted
builtin/grep.c: integrate with sparse index
Turn on sparse index and remove ensure_full_index(). Before this patch, `git-grep` utilizes the ensure_full_index() method to expand the index and search all the entries. Because this method requires walking all the trees and constructing the index, it is the slow part within the whole command. To achieve better performance, this patch uses grep_tree() to search the sparse directory entries and get rid of the ensure_full_index() method. Why grep_tree() is a better choice over ensure_full_index()? 1) grep_tree() is as correct as ensure_full_index(). grep_tree() looks into every sparse-directory entry (represented by a tree) recursively when looping over the index, and the result of doing so matches the result of expanding the index. 2) grep_tree() utilizes pathspecs to limit the scope of searching. ensure_full_index() always expands the index, which means it will always walk all the trees and blobs in the repo without caring if the user only wants a subset of the content, i.e. using a pathspec. On the other hand, grep_tree() will only search the contents that match the pathspec, and thus possibly walking fewer trees. 3) grep_tree() does not construct and copy back a new index, while ensure_full_index() does. This also saves some time. ---------------- Performance test - Summary: p2000 tests demonstrate a ~71% execution time reduction for `git grep --cached bogus -- "f2/f1/f1/*"` using tree-walking logic. However, notice that this result varies depending on the pathspec given. See below "Command used for testing" for more details. Test HEAD~ HEAD ------------------------------------------------------- 2000.78: git grep ... (full-v3) 0.35 0.39 (≈) 2000.79: git grep ... (full-v4) 0.36 0.30 (≈) 2000.80: git grep ... (sparse-v3) 0.88 0.23 (-73.8%) 2000.81: git grep ... (sparse-v4) 0.83 0.26 (-68.6%) - Command used for testing: git grep --cached bogus -- "f2/f1/f1/*" The reason for specifying a pathspec is that, if we don't specify a pathspec, then grep_tree() will walk all the trees and blobs to find the pattern, and the time consumed doing so is not too different from using the original ensure_full_index() method, which also spends most of the time walking trees. However, when a pathspec is specified, this latest logic will only walk the area of trees enclosed by the pathspec, and the time consumed is reasonably a lot less. Generally speaking, because the performance gain is acheived by walking less trees, which are specified by the pathspec, the HEAD time v.s. HEAD~ time in sparse-v[3|4], should be proportional to "pathspec enclosed area" v.s. "all area", respectively. Namely, the wider the <pathspec> is encompassing, the less the performance difference between HEAD~ and HEAD, and vice versa. That is, if we don't specify a pathspec, the performance difference [1] is indistinguishable: both methods walk all the trees and take generally same amount of time (even with the index construction time included for ensure_full_index()). [1] Performance test result without pathspec (hence walking all trees): Command used: git grep --cached bogus Test HEAD~ HEAD --------------------------------------------------- 2000.78: git grep ... (full-v3) 6.17 5.19 (≈) 2000.79: git grep ... (full-v4) 6.19 5.46 (≈) 2000.80: git grep ... (sparse-v3) 6.57 6.44 (≈) 2000.81: git grep ... (sparse-v4) 6.65 6.28 (≈) -------------------------- NEEDSWORK about submodules There are a few NEEDSWORKs that belong to improvements beyond this topic. See the NEEDSWORK in builtin/grep.c::grep_submodule() for more context. The other two NEEDSWORKs in t1092 are also relative. Suggested-by: Derrick Stolee <[email protected]> Helped-by: Derrick Stolee <[email protected]> Helped-by: Victoria Dye <[email protected]> Helped-by: Elijah Newren <[email protected]> Signed-off-by: Shaoxuan Yuan <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 07ee72d commit 7cae762

File tree

3 files changed

+118
-3
lines changed

3 files changed

+118
-3
lines changed

builtin/grep.c

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -458,6 +458,33 @@ static int grep_submodule(struct grep_opt *opt,
458458
* subrepo's odbs to the in-memory alternates list.
459459
*/
460460
obj_read_lock();
461+
462+
/*
463+
* NEEDSWORK: when reading a submodule, the sparsity settings in the
464+
* superproject are incorrectly forgotten or misused. For example:
465+
*
466+
* 1. "command_requires_full_index"
467+
* When this setting is turned on for `grep`, only the superproject
468+
* knows it. All the submodules are read with their own configs
469+
* and get prepare_repo_settings()'d. Therefore, these submodules
470+
* "forget" the sparse-index feature switch. As a result, the index
471+
* of these submodules are expanded unexpectedly.
472+
*
473+
* 2. "core_apply_sparse_checkout"
474+
* When running `grep` in the superproject, this setting is
475+
* populated using the superproject's configs. However, once
476+
* initialized, this config is globally accessible and is read by
477+
* prepare_repo_settings() for the submodules. For instance, if a
478+
* submodule is using a sparse-checkout, however, the superproject
479+
* is not, the result is that the config from the superproject will
480+
* dictate the behavior for the submodule, making it "forget" its
481+
* sparse-checkout state.
482+
*
483+
* 3. "core_sparse_checkout_cone"
484+
* ditto.
485+
*
486+
* Note that this list is not exhaustive.
487+
*/
461488
repo_read_gitmodules(subrepo, 0);
462489

463490
/*
@@ -520,8 +547,6 @@ static int grep_cache(struct grep_opt *opt,
520547
if (repo_read_index(repo) < 0)
521548
die(_("index file corrupt"));
522549

523-
/* TODO: audit for interaction with sparse-index. */
524-
ensure_full_index(repo->index);
525550
for (nr = 0; nr < repo->index->cache_nr; nr++) {
526551
const struct cache_entry *ce = repo->index->cache[nr];
527552

@@ -530,8 +555,20 @@ static int grep_cache(struct grep_opt *opt,
530555

531556
strbuf_setlen(&name, name_base_len);
532557
strbuf_addstr(&name, ce->name);
558+
if (S_ISSPARSEDIR(ce->ce_mode)) {
559+
enum object_type type;
560+
struct tree_desc tree;
561+
void *data;
562+
unsigned long size;
533563

534-
if (S_ISREG(ce->ce_mode) &&
564+
data = read_object_file(&ce->oid, &type, &size);
565+
init_tree_desc(&tree, data, size);
566+
567+
hit |= grep_tree(opt, pathspec, &tree, &name, 0, 0);
568+
strbuf_setlen(&name, name_base_len);
569+
strbuf_addstr(&name, ce->name);
570+
free(data);
571+
} else if (S_ISREG(ce->ce_mode) &&
535572
match_pathspec(repo->index, pathspec, name.buf, name.len, 0, NULL,
536573
S_ISDIR(ce->ce_mode) ||
537574
S_ISGITLINK(ce->ce_mode))) {
@@ -984,6 +1021,11 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
9841021
PARSE_OPT_KEEP_DASHDASH |
9851022
PARSE_OPT_STOP_AT_NON_OPTION);
9861023

1024+
if (the_repository->gitdir) {
1025+
prepare_repo_settings(the_repository);
1026+
the_repository->settings.command_requires_full_index = 0;
1027+
}
1028+
9871029
if (use_index && !startup_info->have_repository) {
9881030
int fallback = 0;
9891031
git_config_get_bool("grep.fallbacktonoindex", &fallback);

t/perf/p2000-sparse-operations.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,5 +124,6 @@ test_perf_on_all git read-tree -mu HEAD
124124
test_perf_on_all git checkout-index -f --all
125125
test_perf_on_all git update-index --add --remove $SPARSE_CONE/a
126126
test_perf_on_all "git rm -f $SPARSE_CONE/a && git checkout HEAD -- $SPARSE_CONE/a"
127+
test_perf_on_all git grep --cached --sparse bogus -- "f2/f1/f1/*"
127128

128129
test_done

t/t1092-sparse-checkout-compatibility.sh

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,19 @@ init_repos () {
162162
git -C sparse-index sparse-checkout set deep
163163
}
164164

165+
init_repos_as_submodules () {
166+
git reset --hard &&
167+
init_repos &&
168+
git submodule add ./full-checkout &&
169+
git submodule add ./sparse-checkout &&
170+
git submodule add ./sparse-index &&
171+
172+
git submodule status >actual &&
173+
grep full-checkout actual &&
174+
grep sparse-checkout actual &&
175+
grep sparse-index actual
176+
}
177+
165178
run_on_sparse () {
166179
(
167180
cd sparse-checkout &&
@@ -1972,4 +1985,63 @@ test_expect_success 'sparse index is not expanded: rm' '
19721985
ensure_not_expanded rm -r deep
19731986
'
19741987

1988+
test_expect_success 'grep with and --cached' '
1989+
init_repos &&
1990+
1991+
test_all_match git grep --cached a &&
1992+
test_all_match git grep --cached a -- "folder1/*"
1993+
'
1994+
1995+
test_expect_success 'grep is not expanded' '
1996+
init_repos &&
1997+
1998+
ensure_not_expanded grep a &&
1999+
ensure_not_expanded grep a -- deep/* &&
2000+
2001+
# All files within the folder1/* pathspec are sparse,
2002+
# so this command does not find any matches
2003+
ensure_not_expanded ! grep a -- folder1/* &&
2004+
2005+
# test out-of-cone pathspec with or without wildcard
2006+
ensure_not_expanded grep --cached a -- "folder1/a" &&
2007+
ensure_not_expanded grep --cached a -- "folder1/*" &&
2008+
2009+
# test in-cone pathspec with or without wildcard
2010+
ensure_not_expanded grep --cached a -- "deep/a" &&
2011+
ensure_not_expanded grep --cached a -- "deep/*"
2012+
'
2013+
2014+
# NEEDSWORK: when running `grep` in the superproject with --recurse-submodules,
2015+
# Git expands the index of the submodules unexpectedly. Even though `grep`
2016+
# builtin is marked as "command_requires_full_index = 0", this config is only
2017+
# useful for the superproject. Namely, the submodules have their own configs,
2018+
# which are _not_ populated by the one-time sparse-index feature switch.
2019+
test_expect_failure 'grep within submodules is not expanded' '
2020+
init_repos_as_submodules &&
2021+
2022+
# do not use ensure_not_expanded() here, becasue `grep` should be
2023+
# run in the superproject, not in "./sparse-index"
2024+
GIT_TRACE2_EVENT="$(pwd)/trace2.txt" \
2025+
git grep --cached --recurse-submodules a -- "*/folder1/*" &&
2026+
test_region ! index ensure_full_index trace2.txt
2027+
'
2028+
2029+
# NEEDSWORK: this test is not actually testing the code. The design purpose
2030+
# of this test is to verify the grep result when the submodules are using a
2031+
# sparse-index. Namely, we want "folder1/" as a tree (a sparse directory); but
2032+
# because of the index expansion, we are now grepping the "folder1/a" blob.
2033+
# Because of the problem stated above 'grep within submodules is not expanded',
2034+
# we don't have the ideal test environment yet.
2035+
test_expect_success 'grep sparse directory within submodules' '
2036+
init_repos_as_submodules &&
2037+
2038+
cat >expect <<-\EOF &&
2039+
full-checkout/folder1/a:a
2040+
sparse-checkout/folder1/a:a
2041+
sparse-index/folder1/a:a
2042+
EOF
2043+
git grep --cached --recurse-submodules a -- "*/folder1/*" >actual &&
2044+
test_cmp actual expect
2045+
'
2046+
19752047
test_done

0 commit comments

Comments
 (0)