[PATCH 0/3] Integrate the sparse index with 'git apply' and 'git add -p/-i'

[PATCH 0/3] Integrate the sparse index with 'git apply' and 'git add -p/-i'

[Derrick Stolee via GitGitGadget]

The sparse index helps make some Git commands faster when using
sparse-checkout in cone mode. However, not all code paths are aware that the
index can have non-blob entries, so we are careful about rolling this
feature out gradually. The cost of this rollout is that some commands are
slower with the sparse index as they need to expand a sparse index into a
full index in memory, which requires parsing tree objects to construct the
full path list.

This patch series focuses on the 'git add -p' command, which is slow with
the sparse index for a couple of reasons, handled in the first two patches:

 1. 'git add -p' uses 'git apply' as a subcommand and 'git apply' needs
    integration with the sparse index. Luckily, we just need to add the repo
    setting and appropriate tests to confirm it behaves as expected.
 2. The interactive modes of 'git add' ('-p' and '-i') leave cmd_add()
    before the code that sets the repo setting to allow for a sparse index.
    Patch 2 fixes this and adds appropriate tests to confirm the behavior in
    a sparse-checkout.

A third patch adds a performance test to p2000-sparse-operations.sh to
confirm that we are getting the performance improvement we expect:

                        BASE    PATCH 1        PATCH 2
---------------------------------------------------------
2000.118: (full-v3)     0.80   0.84 +5.0%     0.84  +5.0%
2000.119: (full-v4)     0.76   0.79 +3.9%     0.80  +5.3%
2000.120: (sparse-v3)   2.09   1.39 -33.5%    0.07 -96.7%
2000.121: (sparse-v4)   2.09   1.39 -33.5%    0.07 -96.7%


Thanks, -Stolee

Derrick Stolee (3):
  apply: integrate with the sparse index
  git add: make -p/-i aware of sparse index
  p2000: add performance test for 'git add -p'

 builtin/add.c                            |   7 +-
 builtin/apply.c                          |   7 +-
 t/perf/p2000-sparse-operations.sh        |   1 +
 t/t1092-sparse-checkout-compatibility.sh | 102 +++++++++++++++++++++++
 4 files changed, 113 insertions(+), 4 deletions(-)


base-commit: 6c0bd1fc70efaf053abe4e57c976afdc72d15377
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1914%2Fderrickstolee%2Fapply-sparse-index-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1914/derrickstolee/apply-sparse-index-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1914
-- 
gitgitgadget

[PATCH 1/3] apply: integrate with the sparse index

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

The sparse index allows storing directory entries in the index, marked
with the skip-wortkree bit and pointing to a tree object. This may be an
unexpected data shape for some implementation areas, so we are rolling
it out incrementally on a builtin-per-builtin basis.

This change enables the sparse index for 'git apply'. The main
motivation for this change is that 'git apply' is used as a child
process of 'git add -p' and expanding the sparse index for each of those
child processes can lead to significant performance issues.

The good news is that the actual index manipulation code used by 'git
apply' is already integrated with the sparse index, so the only product
change is to mark the builtin as allowing the sparse index so it isn't
inflated on read.

The more involved part of this change is around adding tests that verify
how 'git apply' behaves in a sparse-checkout environment and whether or
not the index expands in certain operations.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 builtin/apply.c                          |  7 +++-
 t/t1092-sparse-checkout-compatibility.sh | 46 ++++++++++++++++++++++++
 2 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index 84f1863d3ac3..a1e20c593d09 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -12,7 +12,7 @@ static const char * const apply_usage[] = {
 int cmd_apply(int argc,
 	      const char **argv,
 	      const char *prefix,
-	      struct repository *repo UNUSED)
+	      struct repository *repo)
 {
 	int force_apply = 0;
 	int options = 0;
@@ -35,6 +35,11 @@ int cmd_apply(int argc,
 				   &state, &force_apply, &options,
 				   apply_usage);
 
+	if (repo) {
+		prepare_repo_settings(repo);
+		repo->settings.command_requires_full_index = 0;
+	}
+
 	if (check_apply_state(&state, force_apply))
 		exit(128);
 
diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh
index f9b448792cb4..ab8bd371eff3 100755
--- a/t/t1092-sparse-checkout-compatibility.sh
+++ b/t/t1092-sparse-checkout-compatibility.sh
@@ -1340,6 +1340,30 @@ test_expect_success 'submodule handling' '
 	grep "160000 $(git -C initial-repo rev-parse HEAD) 0	modules/sub" cache
 '
 
+test_expect_success 'git apply functionality' '
+	init_repos &&
+
+	test_all_match git checkout base &&
+
+	git -C full-checkout diff base..merge-right -- deep >patch-in-sparse &&
+	git -C full-checkout diff base..merge-right -- folder2 >patch-outside &&
+
+	# Apply a patch to a file inside the sparse definition
+	test_all_match git apply --index --stat ../patch-in-sparse &&
+	test_all_match git status --porcelain=v2 &&
+
+	# Apply a patch to a file outside the sparse definition
+	test_sparse_match test_must_fail git apply ../patch-outside &&
+	grep "No such file or directory" sparse-checkout-err &&
+
+	# But it works with --index and --cached
+	test_all_match git apply --index --stat ../patch-outside &&
+	test_all_match git status --porcelain=v2 &&
+	test_all_match git reset --hard &&
+	test_all_match git apply --cached --stat ../patch-outside &&
+	test_all_match git status --porcelain=v2
+'
+
 # When working with a sparse index, some commands will need to expand the
 # index to operate properly. If those commands also write the index back
 # to disk, they need to convert the index to sparse before writing.
@@ -2345,6 +2369,28 @@ test_expect_success 'sparse-index is not expanded: check-attr' '
 	ensure_not_expanded check-attr -a --cached -- folder1/a
 '
 
+test_expect_success 'sparse-index is not expanded: git apply' '
+	init_repos &&
+
+	git -C sparse-index checkout base &&
+	git -C full-checkout diff base..merge-right -- deep >patch-in-sparse &&
+	git -C full-checkout diff base..merge-right -- folder2 >patch-outside &&
+
+	# Apply a patch to a file inside the sparse definition
+	ensure_not_expanded apply --index --stat ../patch-in-sparse &&
+
+	# Apply a patch to a file outside the sparse definition
+	# Fails when caring about the worktree.
+	ensure_not_expanded ! apply ../patch-outside &&
+
+	# Expands when using --index.
+	ensure_expanded apply --index ../patch-outside &&
+	git -C sparse-index reset --hard &&
+
+	# Does not expand when using --cached.
+	ensure_not_expanded apply --cached ../patch-outside
+'
+
 test_expect_success 'advice.sparseIndexExpanded' '
 	init_repos &&
 
-- 
gitgitgadget

Re: [PATCH 1/3] apply: integrate with the sparse index

[Elijah Newren]

On Tue, May 6, 2025 at 5:55 PM Derrick Stolee via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Derrick Stolee <stolee@gmail.com>
>
> The sparse index allows storing directory entries in the index, marked
> with the skip-wortkree bit and pointing to a tree object. This may be an
> unexpected data shape for some implementation areas, so we are rolling
> it out incrementally on a builtin-per-builtin basis.
>
> This change enables the sparse index for 'git apply'. The main
> motivation for this change is that 'git apply' is used as a child
> process of 'git add -p' and expanding the sparse index for each of those
> child processes can lead to significant performance issues.
>
> The good news is that the actual index manipulation code used by 'git
> apply' is already integrated with the sparse index, so the only product
> change is to mark the builtin as allowing the sparse index so it isn't
> inflated on read.
>
> The more involved part of this change is around adding tests that verify
> how 'git apply' behaves in a sparse-checkout environment and whether or
> not the index expands in certain operations.
>
> Signed-off-by: Derrick Stolee <stolee@gmail.com>
> ---
>  builtin/apply.c                          |  7 +++-
>  t/t1092-sparse-checkout-compatibility.sh | 46 ++++++++++++++++++++++++
>  2 files changed, 52 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/apply.c b/builtin/apply.c
> index 84f1863d3ac3..a1e20c593d09 100644
> --- a/builtin/apply.c
> +++ b/builtin/apply.c
> @@ -12,7 +12,7 @@ static const char * const apply_usage[] = {
>  int cmd_apply(int argc,
>               const char **argv,
>               const char *prefix,
> -             struct repository *repo UNUSED)
> +             struct repository *repo)
>  {
>         int force_apply = 0;
>         int options = 0;
> @@ -35,6 +35,11 @@ int cmd_apply(int argc,
>                                    &state, &force_apply, &options,
>                                    apply_usage);
>
> +       if (repo) {
> +               prepare_repo_settings(repo);
> +               repo->settings.command_requires_full_index = 0;
> +       }
> +
>         if (check_apply_state(&state, force_apply))
>                 exit(128);
>
> diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh
> index f9b448792cb4..ab8bd371eff3 100755
> --- a/t/t1092-sparse-checkout-compatibility.sh
> +++ b/t/t1092-sparse-checkout-compatibility.sh
> @@ -1340,6 +1340,30 @@ test_expect_success 'submodule handling' '
>         grep "160000 $(git -C initial-repo rev-parse HEAD) 0    modules/sub" cache
>  '
>
> +test_expect_success 'git apply functionality' '
> +       init_repos &&
> +
> +       test_all_match git checkout base &&
> +
> +       git -C full-checkout diff base..merge-right -- deep >patch-in-sparse &&
> +       git -C full-checkout diff base..merge-right -- folder2 >patch-outside &&
> +
> +       # Apply a patch to a file inside the sparse definition
> +       test_all_match git apply --index --stat ../patch-in-sparse &&
> +       test_all_match git status --porcelain=v2 &&
> +
> +       # Apply a patch to a file outside the sparse definition
> +       test_sparse_match test_must_fail git apply ../patch-outside &&
> +       grep "No such file or directory" sparse-checkout-err &&

I was slightly confused by this at first, because I was thinking of
the case where folder2/a exists in the working directory despite not
matching the sparsity patterns, but you were testing a case where the
working directory matched the sparsity patterns, so folder2/a doesn't
exist.

So, the check here looks good.

And, when folder2/a does exist, then we're in the case handled by
82386b44963f (Merge branch 'en/present-despite-skipped', 2022-03-09),
which forces the directory to not be considered sparse, and so it's
just like the ../patch-in-sparse case...meaning it's not really a
different case to test.

So, it all makes sense.

> +
> +       # But it works with --index and --cached
> +       test_all_match git apply --index --stat ../patch-outside &&
> +       test_all_match git status --porcelain=v2 &&
> +       test_all_match git reset --hard &&
> +       test_all_match git apply --cached --stat ../patch-outside &&
> +       test_all_match git status --porcelain=v2
> +'
> +
>  # When working with a sparse index, some commands will need to expand the
>  # index to operate properly. If those commands also write the index back
>  # to disk, they need to convert the index to sparse before writing.
> @@ -2345,6 +2369,28 @@ test_expect_success 'sparse-index is not expanded: check-attr' '
>         ensure_not_expanded check-attr -a --cached -- folder1/a
>  '
>
> +test_expect_success 'sparse-index is not expanded: git apply' '
> +       init_repos &&
> +
> +       git -C sparse-index checkout base &&
> +       git -C full-checkout diff base..merge-right -- deep >patch-in-sparse &&
> +       git -C full-checkout diff base..merge-right -- folder2 >patch-outside &&
> +
> +       # Apply a patch to a file inside the sparse definition
> +       ensure_not_expanded apply --index --stat ../patch-in-sparse &&
> +
> +       # Apply a patch to a file outside the sparse definition
> +       # Fails when caring about the worktree.
> +       ensure_not_expanded ! apply ../patch-outside &&
> +
> +       # Expands when using --index.
> +       ensure_expanded apply --index ../patch-outside &&
> +       git -C sparse-index reset --hard &&

All makes sense up to here.

> +
> +       # Does not expand when using --cached.
> +       ensure_not_expanded apply --cached ../patch-outside

Wait, what?  That makes no sense.

After some digging, I see why the test passed, but it's very
misleading.  Just before this command, if you ran the following
commands from the sparse-index directory, you'd see the following:

$ rm testme
$ GIT_TRACE2_EVENT=$(pwd)/testme git apply --cached ../patch-outside
$ grep ensure_full_index testme
$

Which matches what you were testing and shows why it passed for you.
But I'd argue the test is not correct and confusing for anyone that
reads it, because:

$ git ls-files -s --sparse | grep folder2
040000 cb4007891397aa2a451037d1c69e57f0cf498c24 0    folder2/0/
100644 78981922613b2afb6025042ff6bd878ac1994e85 0    folder2/a

In other words, the index was *already* (partially) expanded by the
`git apply --index`, and the `git reset --hard` did not fix that
contrary to expectations.  Continuing from here we see:

$ git reset --hard
HEAD is now at 703fd3e initial commit
$ git ls-files -s --sparse | grep folder2
040000 cb4007891397aa2a451037d1c69e57f0cf498c24 0    folder2/0/
100644 78981922613b2afb6025042ff6bd878ac1994e85 0    folder2/a
$ git sparse-checkout reapply
$ git ls-files -s --sparse | grep folder2
040000 123706f6fc38949628eaf0483edbf97ba21123ae 0    folder2/

So, we need to do a `git sparse-checkout reapply` to make sure we were
actually in the expected fully sparse state.  From here...

$ rm testme
$ GIT_TRACE2_EVENT=$(pwd)/testme git apply --cached ../patch-outside
$ grep ensure_full_index testme
{"event":"region_enter","sid":"20250510T030856.854763Z-H8ec63c79-P0000371c","thread":"main","time":"2025-05-10T03:08:56.856008Z","file":"sparse-index.c","line":372,"repo":1,"nesting":1,"category":"index","label":"ensure_full_index"}
{"event":"region_leave","sid":"20250510T030856.854763Z-H8ec63c79-P0000371c","thread":"main","time":"2025-05-10T03:08:56.856454Z","file":"sparse-index.c","line":455,"repo":1,"t_rel":0.000446,"nesting":1,"category":"index","label":"ensure_full_index"}
{"event":"region_enter","sid":"20250510T030856.854763Z-H8ec63c79-P0000371c","thread":"main","time":"2025-05-10T03:08:56.857016Z","file":"sparse-index.c","line":372,"repo":1,"nesting":1,"category":"index","label":"ensure_full_index"}
{"event":"region_leave","sid":"20250510T030856.854763Z-H8ec63c79-P0000371c","thread":"main","time":"2025-05-10T03:08:56.857135Z","file":"sparse-index.c","line":455,"repo":1,"t_rel":0.000119,"nesting":1,"category":"index","label":"ensure_full_index"}

So, indeed, `git apply --cached ../patch-outside` DOES expand the
index, as I expected.  It has to when folder2/ is a directory in the
index, so that we can get a folder2/a entry that we can modify.  And
that's just what we see:

$ git ls-files -s --sparse | grep folder2
040000 cb4007891397aa2a451037d1c69e57f0cf498c24 0    folder2/0/
100644 50f4ca4e6265a0497ec2ee6782648138914ad398 0    folder2/a


Can you add a `git sparse-checkout reapply` right after your `git
reset --hard`, and then switch the ensure_not_expanded to
ensure_expanded for the apply --cached call?

Re: [PATCH 1/3] apply: integrate with the sparse index

[Derrick Stolee]

On 5/9/25 11:18 PM, Elijah Newren wrote:
> On Tue, May 6, 2025 at 5:55 PM Derrick Stolee via GitGitGadget


>> +       # Expands when using --index.
>> +       ensure_expanded apply --index ../patch-outside &&
>> +       git -C sparse-index reset --hard &&
> 
> All makes sense up to here.
> 
>> +
>> +       # Does not expand when using --cached.
>> +       ensure_not_expanded apply --cached ../patch-outside
> 
> Wait, what?  That makes no sense.
> 
> After some digging, I see why the test passed, but it's very
> misleading.  Just before this command, if you ran the following
> commands from the sparse-index directory, you'd see the following:
> 
> $ rm testme
> $ GIT_TRACE2_EVENT=$(pwd)/testme git apply --cached ../patch-outside
> $ grep ensure_full_index testme
> $
> 
> Which matches what you were testing and shows why it passed for you.
> But I'd argue the test is not correct and confusing for anyone that
> reads it, because:
> 
> $ git ls-files -s --sparse | grep folder2
> 040000 cb4007891397aa2a451037d1c69e57f0cf498c24 0    folder2/0/
> 100644 78981922613b2afb6025042ff6bd878ac1994e85 0    folder2/a
> 
> In other words, the index was *already* (partially) expanded by the
> `git apply --index`, and the `git reset --hard` did not fix that
> contrary to expectations.  Continuing from here we see:
> 
> $ git reset --hard
> HEAD is now at 703fd3e initial commit
> $ git ls-files -s --sparse | grep folder2
> 040000 cb4007891397aa2a451037d1c69e57f0cf498c24 0    folder2/0/
> 100644 78981922613b2afb6025042ff6bd878ac1994e85 0    folder2/a
> $ git sparse-checkout reapply
> $ git ls-files -s --sparse | grep folder2
> 040000 123706f6fc38949628eaf0483edbf97ba21123ae 0    folder2/
> 
> So, we need to do a `git sparse-checkout reapply` to make sure we were
> actually in the expected fully sparse state.  From here...
> 
> $ rm testme
> $ GIT_TRACE2_EVENT=$(pwd)/testme git apply --cached ../patch-outside
> $ grep ensure_full_index testme
> {"event":"region_enter","sid":"20250510T030856.854763Z-H8ec63c79-P0000371c","thread":"main","time":"2025-05-10T03:08:56.856008Z","file":"sparse-index.c","line":372,"repo":1,"nesting":1,"category":"index","label":"ensure_full_index"}
> {"event":"region_leave","sid":"20250510T030856.854763Z-H8ec63c79-P0000371c","thread":"main","time":"2025-05-10T03:08:56.856454Z","file":"sparse-index.c","line":455,"repo":1,"t_rel":0.000446,"nesting":1,"category":"index","label":"ensure_full_index"}
> {"event":"region_enter","sid":"20250510T030856.854763Z-H8ec63c79-P0000371c","thread":"main","time":"2025-05-10T03:08:56.857016Z","file":"sparse-index.c","line":372,"repo":1,"nesting":1,"category":"index","label":"ensure_full_index"}
> {"event":"region_leave","sid":"20250510T030856.854763Z-H8ec63c79-P0000371c","thread":"main","time":"2025-05-10T03:08:56.857135Z","file":"sparse-index.c","line":455,"repo":1,"t_rel":0.000119,"nesting":1,"category":"index","label":"ensure_full_index"}
> 
> So, indeed, `git apply --cached ../patch-outside` DOES expand the
> index, as I expected.  It has to when folder2/ is a directory in the
> index, so that we can get a folder2/a entry that we can modify.  And
> that's just what we see:
> 
> $ git ls-files -s --sparse | grep folder2
> 040000 cb4007891397aa2a451037d1c69e57f0cf498c24 0    folder2/0/
> 100644 50f4ca4e6265a0497ec2ee6782648138914ad398 0    folder2/a
> 
> 
> Can you add a `git sparse-checkout reapply` right after your `git
> reset --hard`, and then switch the ensure_not_expanded to
> ensure_expanded for the apply --cached call?

Thanks for digging in here. I'm going to augment the test to
include both the ensure_expanded and ensure_not_expanded so we
get the extra coverage for these two scenarios.

Thanks for the careful eye!
-Stolee

[PATCH 2/3] git add: make -p/-i aware of sparse index

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

It is slow to expand a sparse index in-memory due to parsing of trees.
We aim to minimize that performance cost when possible. 'git add -p'
uses 'git apply' child processes to modify the index, but still there
are some expansions that occur.

It turns out that control flows out of cmd_add() in the interactive
cases before the lines that confirm that the builtin is integrated with
the sparse index. We need to move that earlier to ensure it prevents a
full index expansion on read.

Add more test cases that confirm that these interactive add options work
with the sparse index. One interesting aspect here is that the '-i'
option avoids expanding the sparse index when a sparse directory exists
on disk while the '-p' option does hit the ensure_full_index() method.
This leaves some room for improvement, but this case should be atypical
as users should remain within their sparse-checkout.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 builtin/add.c                            |  7 +--
 t/t1092-sparse-checkout-compatibility.sh | 56 ++++++++++++++++++++++++
 2 files changed, 60 insertions(+), 3 deletions(-)

diff --git a/builtin/add.c b/builtin/add.c
index 747511b68bc3..7c292ffdc6c2 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -390,6 +390,10 @@ int cmd_add(int argc,
 
 	argc = parse_options(argc, argv, prefix, builtin_add_options,
 			  builtin_add_usage, PARSE_OPT_KEEP_ARGV0);
+
+	prepare_repo_settings(repo);
+	repo->settings.command_requires_full_index = 0;
+
 	if (patch_interactive)
 		add_interactive = 1;
 	if (add_interactive) {
@@ -426,9 +430,6 @@ int cmd_add(int argc,
 	add_new_files = !take_worktree_changes && !refresh_only && !add_renormalize;
 	require_pathspec = !(take_worktree_changes || (0 < addremove_explicit));
 
-	prepare_repo_settings(repo);
-	repo->settings.command_requires_full_index = 0;
-
 	repo_hold_locked_index(repo, &lock_file, LOCK_DIE_ON_ERROR);
 
 	/*
diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh
index ab8bd371eff3..0dc5dd27184d 100755
--- a/t/t1092-sparse-checkout-compatibility.sh
+++ b/t/t1092-sparse-checkout-compatibility.sh
@@ -384,6 +384,38 @@ test_expect_success 'add, commit, checkout' '
 	test_all_match git checkout -
 '
 
+test_expect_success 'git add -p' '
+	init_repos &&
+
+	write_script edit-contents <<-\EOF &&
+	echo text >>$1
+	EOF
+
+	# Does not expand when edits are within sparse checkout.
+	run_on_all ../edit-contents deep/a &&
+	run_on_all ../edit-contents deep/deeper1/a &&
+
+	test_write_lines y n >in &&
+	run_on_all git add -p <in &&
+	test_all_match git status --porcelain=v2 &&
+	test_all_match git reset &&
+
+	test_write_lines u 1 "" q >in &&
+	run_on_all git add -i <in &&
+	test_all_match git status --porcelain=v2 &&
+	test_all_match git reset --hard &&
+
+	run_on_sparse mkdir -p folder1 &&
+	run_on_all ../edit-contents folder1/a &&
+	test_write_lines y n y >in &&
+	run_on_all git add -p <in &&
+	test_sparse_match git status --porcelain=v2 &&
+	test_sparse_match git reset &&
+	test_write_lines u 2 3 "" q >in &&
+	run_on_all git add -i <in &&
+	test_sparse_match git status --porcelain=v2
+'
+
 test_expect_success 'deep changes during checkout' '
 	init_repos &&
 
@@ -2391,6 +2423,30 @@ test_expect_success 'sparse-index is not expanded: git apply' '
 	ensure_not_expanded apply --cached ../patch-outside
 '
 
+test_expect_success 'sparse-index is not expanded: git add -p' '
+	init_repos &&
+
+	# Does not expand when edits are within sparse checkout.
+	echo "new content" >sparse-index/deep/a &&
+	echo "new content" >sparse-index/deep/deeper1/a &&
+	test_write_lines y n >in &&
+	ensure_not_expanded add -p <in &&
+	git -C sparse-index reset &&
+	ensure_not_expanded add -i <in &&
+
+	mkdir -p sparse-index/folder1 &&
+	echo "new content" >sparse-index/folder1/a &&
+
+	# -p does expand when edits are outside sparse checkout.
+	test_write_lines y n y >in &&
+	ensure_expanded add -p <in &&
+
+	# but -i does not expand.
+	git -C sparse-index reset &&
+	test_write_lines u 2 3 "" q >in &&
+	ensure_not_expanded add -i <in
+'
+
 test_expect_success 'advice.sparseIndexExpanded' '
 	init_repos &&
 
-- 
gitgitgadget

Re: [PATCH 2/3] git add: make -p/-i aware of sparse index

[Elijah Newren]

On Tue, May 6, 2025 at 5:55 PM Derrick Stolee via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Derrick Stolee <stolee@gmail.com>
>
> It is slow to expand a sparse index in-memory due to parsing of trees.
> We aim to minimize that performance cost when possible. 'git add -p'
> uses 'git apply' child processes to modify the index, but still there
> are some expansions that occur.

still there are some expansions that occur...outside of those child
processes?  Is that what you're trying to say, or was it something
else?

> It turns out that control flows out of cmd_add() in the interactive
> cases before the lines that confirm that the builtin is integrated with
> the sparse index. We need to move that earlier to ensure it prevents a
> full index expansion on read.
>
> Add more test cases that confirm that these interactive add options work
> with the sparse index. One interesting aspect here is that the '-i'
> option avoids expanding the sparse index when a sparse directory exists
> on disk while the '-p' option does hit the ensure_full_index() method.
> This leaves some room for improvement, but this case should be atypical
> as users should remain within their sparse-checkout.

It's not clear whether this paragraph is talking about existing state
(before the patch) or desired state (after the patch).  I think based
on the context it's the former, but the last sentence sounds more like
a future work direction that makes it very unclear, to me at least.

> Signed-off-by: Derrick Stolee <stolee@gmail.com>
> ---
>  builtin/add.c                            |  7 +--
>  t/t1092-sparse-checkout-compatibility.sh | 56 ++++++++++++++++++++++++
>  2 files changed, 60 insertions(+), 3 deletions(-)
>
> diff --git a/builtin/add.c b/builtin/add.c
> index 747511b68bc3..7c292ffdc6c2 100644
> --- a/builtin/add.c
> +++ b/builtin/add.c
> @@ -390,6 +390,10 @@ int cmd_add(int argc,
>
>         argc = parse_options(argc, argv, prefix, builtin_add_options,
>                           builtin_add_usage, PARSE_OPT_KEEP_ARGV0);
> +
> +       prepare_repo_settings(repo);
> +       repo->settings.command_requires_full_index = 0;
> +
>         if (patch_interactive)
>                 add_interactive = 1;
>         if (add_interactive) {
> @@ -426,9 +430,6 @@ int cmd_add(int argc,
>         add_new_files = !take_worktree_changes && !refresh_only && !add_renormalize;
>         require_pathspec = !(take_worktree_changes || (0 < addremove_explicit));
>
> -       prepare_repo_settings(repo);
> -       repo->settings.command_requires_full_index = 0;
> -
>         repo_hold_locked_index(repo, &lock_file, LOCK_DIE_ON_ERROR);
>
>         /*
> diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh
> index ab8bd371eff3..0dc5dd27184d 100755
> --- a/t/t1092-sparse-checkout-compatibility.sh
> +++ b/t/t1092-sparse-checkout-compatibility.sh
> @@ -384,6 +384,38 @@ test_expect_success 'add, commit, checkout' '
>         test_all_match git checkout -
>  '
>
> +test_expect_success 'git add -p' '
> +       init_repos &&
> +
> +       write_script edit-contents <<-\EOF &&
> +       echo text >>$1
> +       EOF
> +
> +       # Does not expand when edits are within sparse checkout.
> +       run_on_all ../edit-contents deep/a &&
> +       run_on_all ../edit-contents deep/deeper1/a &&
> +
> +       test_write_lines y n >in &&
> +       run_on_all git add -p <in &&
> +       test_all_match git status --porcelain=v2 &&
> +       test_all_match git reset &&
> +
> +       test_write_lines u 1 "" q >in &&
> +       run_on_all git add -i <in &&
> +       test_all_match git status --porcelain=v2 &&
> +       test_all_match git reset --hard &&
> +
> +       run_on_sparse mkdir -p folder1 &&
> +       run_on_all ../edit-contents folder1/a &&
> +       test_write_lines y n y >in &&
> +       run_on_all git add -p <in &&
> +       test_sparse_match git status --porcelain=v2 &&
> +       test_sparse_match git reset &&
> +       test_write_lines u 2 3 "" q >in &&
> +       run_on_all git add -i <in &&
> +       test_sparse_match git status --porcelain=v2
> +'
> +
>  test_expect_success 'deep changes during checkout' '
>         init_repos &&
>
> @@ -2391,6 +2423,30 @@ test_expect_success 'sparse-index is not expanded: git apply' '
>         ensure_not_expanded apply --cached ../patch-outside
>  '
>
> +test_expect_success 'sparse-index is not expanded: git add -p' '
> +       init_repos &&
> +
> +       # Does not expand when edits are within sparse checkout.
> +       echo "new content" >sparse-index/deep/a &&
> +       echo "new content" >sparse-index/deep/deeper1/a &&
> +       test_write_lines y n >in &&
> +       ensure_not_expanded add -p <in &&
> +       git -C sparse-index reset &&
> +       ensure_not_expanded add -i <in &&
> +
> +       mkdir -p sparse-index/folder1 &&
> +       echo "new content" >sparse-index/folder1/a &&
> +
> +       # -p does expand when edits are outside sparse checkout.
> +       test_write_lines y n y >in &&
> +       ensure_expanded add -p <in &&
> +
> +       # but -i does not expand.
> +       git -C sparse-index reset &&
> +       test_write_lines u 2 3 "" q >in &&
> +       ensure_not_expanded add -i <in

This has the same error as patch 1, in that you assume your reset
above (which wasn't even a reset --hard) will re-sparsify the index.
Since it doesn't, your test is misleading and only shows that when
already expanded to include the files of interest it doesn't expand
any further.  To re-sparsify your index before the `add -i` call,
you'll need to do a `git reset --hard && git sparse-checkout reapply`
and then recreate folder1/a with "new content" again...and then run
your 'add -i' command.

Anyway, now I think I understand the expected meaning of the final
paragraph of your commit message, but you were tripped up by assuming
the index was re-sparsified between your steps when it wasn't.

Re: [PATCH 2/3] git add: make -p/-i aware of sparse index

[Derrick Stolee]

On 5/10/25 12:38 AM, Elijah Newren wrote:
> On Tue, May 6, 2025 at 5:55 PM Derrick Stolee via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
>>
>> From: Derrick Stolee <stolee@gmail.com>
>>
>> It is slow to expand a sparse index in-memory due to parsing of trees.
>> We aim to minimize that performance cost when possible. 'git add -p'
>> uses 'git apply' child processes to modify the index, but still there
>> are some expansions that occur.
> 
> still there are some expansions that occur...outside of those child
> processes?  Is that what you're trying to say, or was it something
> else?
> 
>> It turns out that control flows out of cmd_add() in the interactive
>> cases before the lines that confirm that the builtin is integrated with
>> the sparse index. We need to move that earlier to ensure it prevents a
>> full index expansion on read.
>>
>> Add more test cases that confirm that these interactive add options work
>> with the sparse index. One interesting aspect here is that the '-i'
>> option avoids expanding the sparse index when a sparse directory exists
>> on disk while the '-p' option does hit the ensure_full_index() method.
>> This leaves some room for improvement, but this case should be atypical
>> as users should remain within their sparse-checkout.
> 
> It's not clear whether this paragraph is talking about existing state
> (before the patch) or desired state (after the patch).  I think based
> on the context it's the former, but the last sentence sounds more like
> a future work direction that makes it very unclear, to me at least.

I'll try to rewrite to make this clearer.

>> +       # -p does expand when edits are outside sparse checkout.
>> +       test_write_lines y n y >in &&
>> +       ensure_expanded add -p <in &&
>> +
>> +       # but -i does not expand.
>> +       git -C sparse-index reset &&
>> +       test_write_lines u 2 3 "" q >in &&
>> +       ensure_not_expanded add -i <in
> 
> This has the same error as patch 1, in that you assume your reset
> above (which wasn't even a reset --hard) will re-sparsify the index.
> Since it doesn't, your test is misleading and only shows that when
> already expanded to include the files of interest it doesn't expand
> any further.  To re-sparsify your index before the `add -i` call,
> you'll need to do a `git reset --hard && git sparse-checkout reapply`
> and then recreate folder1/a with "new content" again...and then run
> your 'add -i' command.
Thanks. I didn't like that this was different. I appreciate your
expertise helping to clarify this issue.

Thanks,
-Stolee

[PATCH 3/3] p2000: add performance test for 'git add -p'

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

The previous two changes contributed performance improvements to 'git
apply' and 'git add -p' when using a sparse index. Add a performance
test to demonstrate this (and to help validate that performance remains
good in the future).

In the truncated test output below, we see that the full checkout
performance changes within noise expectations, but the sparse index
cases improve 33% and then 96%.

                      HEAD~3     HEAD~2         HEAD~1
---------------------------------------------------------
2000.118: (full-v3)     0.80   0.84 +5.0%     0.84  +5.0%
2000.119: (full-v4)     0.76   0.79 +3.9%     0.80  +5.3%
2000.120: (sparse-v3)   2.09   1.39 -33.5%    0.07 -96.7%
2000.121: (sparse-v4)   2.09   1.39 -33.5%    0.07 -96.7%

It is worth noting that if our test was more involved and had multiple
hunks to evaluate, then the time spent in 'git apply' would dominate due
to multiple index loads and writes. As it stands, we need the sparse
index improvement in 'git add -p' itself to confirm this performance
improvement.

Since the change for 'git add -i' is identical, we avoid a second test
case for that similar operation.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 t/perf/p2000-sparse-operations.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/t/perf/p2000-sparse-operations.sh b/t/perf/p2000-sparse-operations.sh
index 39e92b084143..3da6ae59c000 100755
--- a/t/perf/p2000-sparse-operations.sh
+++ b/t/perf/p2000-sparse-operations.sh
@@ -135,5 +135,6 @@ test_perf_on_all git diff-tree HEAD
 test_perf_on_all git diff-tree HEAD -- $SPARSE_CONE/a
 test_perf_on_all "git worktree add ../temp && git worktree remove ../temp"
 test_perf_on_all git check-attr -a -- $SPARSE_CONE/a
+test_perf_on_all 'echo >>a && test_write_lines y | git add -p'
 
 test_done
-- 
gitgitgadget

Re: [PATCH 3/3] p2000: add performance test for 'git add -p'

[Elijah Newren]

On Tue, May 6, 2025 at 5:55 PM Derrick Stolee via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Derrick Stolee <stolee@gmail.com>
>
> The previous two changes contributed performance improvements to 'git
> apply' and 'git add -p' when using a sparse index. Add a performance
> test to demonstrate this (and to help validate that performance remains
> good in the future).
>
> In the truncated test output below, we see that the full checkout
> performance changes within noise expectations, but the sparse index
> cases improve 33% and then 96%.
>
>                       HEAD~3     HEAD~2         HEAD~1
> ---------------------------------------------------------
> 2000.118: (full-v3)     0.80   0.84 +5.0%     0.84  +5.0%
> 2000.119: (full-v4)     0.76   0.79 +3.9%     0.80  +5.3%
> 2000.120: (sparse-v3)   2.09   1.39 -33.5%    0.07 -96.7%
> 2000.121: (sparse-v4)   2.09   1.39 -33.5%    0.07 -96.7%
>
> It is worth noting that if our test was more involved and had multiple
> hunks to evaluate, then the time spent in 'git apply' would dominate due
> to multiple index loads and writes. As it stands, we need the sparse
> index improvement in 'git add -p' itself to confirm this performance
> improvement.
>
> Since the change for 'git add -i' is identical, we avoid a second test
> case for that similar operation.
>
> Signed-off-by: Derrick Stolee <stolee@gmail.com>
> ---
>  t/perf/p2000-sparse-operations.sh | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/t/perf/p2000-sparse-operations.sh b/t/perf/p2000-sparse-operations.sh
> index 39e92b084143..3da6ae59c000 100755
> --- a/t/perf/p2000-sparse-operations.sh
> +++ b/t/perf/p2000-sparse-operations.sh
> @@ -135,5 +135,6 @@ test_perf_on_all git diff-tree HEAD
>  test_perf_on_all git diff-tree HEAD -- $SPARSE_CONE/a
>  test_perf_on_all "git worktree add ../temp && git worktree remove ../temp"
>  test_perf_on_all git check-attr -a -- $SPARSE_CONE/a
> +test_perf_on_all 'echo >>a && test_write_lines y | git add -p'
>
>  test_done
> --
> gitgitgadget

Very nice!

Re: [PATCH 0/3] Integrate the sparse index with 'git apply' and 'git add -p/-i'

[Junio C Hamano]

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> The sparse index helps make some Git commands faster when using
> sparse-checkout in cone mode. However, not all code paths are aware that the
> index can have non-blob entries, so we are careful about rolling this
> feature out gradually. The cost of this rollout is that some commands are
> slower with the sparse index as they need to expand a sparse index into a
> full index in memory, which requires parsing tree objects to construct the
> full path list.
>
> This patch series focuses on the 'git add -p' command, which is slow with
> the sparse index for a couple of reasons, handled in the first two patches:
>
>  1. 'git add -p' uses 'git apply' as a subcommand and 'git apply' needs
>     integration with the sparse index. Luckily, we just need to add the repo
>     setting and appropriate tests to confirm it behaves as expected.
>  2. The interactive modes of 'git add' ('-p' and '-i') leave cmd_add()
>     before the code that sets the repo setting to allow for a sparse index.
>     Patch 2 fixes this and adds appropriate tests to confirm the behavior in
>     a sparse-checkout.
>
> A third patch adds a performance test to p2000-sparse-operations.sh to
> confirm that we are getting the performance improvement we expect:
>
>                         BASE    PATCH 1        PATCH 2
> ---------------------------------------------------------
> 2000.118: (full-v3)     0.80   0.84 +5.0%     0.84  +5.0%
> 2000.119: (full-v4)     0.76   0.79 +3.9%     0.80  +5.3%
> 2000.120: (sparse-v3)   2.09   1.39 -33.5%    0.07 -96.7%
> 2000.121: (sparse-v4)   2.09   1.39 -33.5%    0.07 -96.7%
>
>
> Thanks, -Stolee

As always, it is delight to read a well-written cover letter that
naturally convinces readers why the series is worth reading ;-)

Thanks.

Re: [PATCH 0/3] Integrate the sparse index with 'git apply' and 'git add -p/-i'

[Phillip Wood]

Hi Stolee

On 07/05/2025 01:55, Derrick Stolee via GitGitGadget wrote:
> The sparse index helps make some Git commands faster when using
> sparse-checkout in cone mode. However, not all code paths are aware that the
> index can have non-blob entries, so we are careful about rolling this
> feature out gradually. The cost of this rollout is that some commands are
> slower with the sparse index as they need to expand a sparse index into a
> full index in memory, which requires parsing tree objects to construct the
> full path list.
> 
> This patch series focuses on the 'git add -p' command, which is slow with
> the sparse index for a couple of reasons, handled in the first two patches:
> 
>   1. 'git add -p' uses 'git apply' as a subcommand and 'git apply' needs
>      integration with the sparse index. Luckily, we just need to add the repo
>      setting and appropriate tests to confirm it behaves as expected.
>   2. The interactive modes of 'git add' ('-p' and '-i') leave cmd_add()
>      before the code that sets the repo setting to allow for a sparse index.
>      Patch 2 fixes this and adds appropriate tests to confirm the behavior in
>      a sparse-checkout.

This made me wonder about the other commands that take "--patch" like 
checkout and reset. Do you know how well they handle the sparse index? 
They'll all benefit from the changes to git apply in this series but I 
was wondering if they need any further changes.

Best Wishes

Phillip

> A third patch adds a performance test to p2000-sparse-operations.sh to
> confirm that we are getting the performance improvement we expect:
> 
>                          BASE    PATCH 1        PATCH 2
> ---------------------------------------------------------
> 2000.118: (full-v3)     0.80   0.84 +5.0%     0.84  +5.0%
> 2000.119: (full-v4)     0.76   0.79 +3.9%     0.80  +5.3%
> 2000.120: (sparse-v3)   2.09   1.39 -33.5%    0.07 -96.7%
> 2000.121: (sparse-v4)   2.09   1.39 -33.5%    0.07 -96.7%
> 
> 
> Thanks, -Stolee
> 
> Derrick Stolee (3):
>    apply: integrate with the sparse index
>    git add: make -p/-i aware of sparse index
>    p2000: add performance test for 'git add -p'
> 
>   builtin/add.c                            |   7 +-
>   builtin/apply.c                          |   7 +-
>   t/perf/p2000-sparse-operations.sh        |   1 +
>   t/t1092-sparse-checkout-compatibility.sh | 102 +++++++++++++++++++++++
>   4 files changed, 113 insertions(+), 4 deletions(-)
> 
> 
> base-commit: 6c0bd1fc70efaf053abe4e57c976afdc72d15377
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1914%2Fderrickstolee%2Fapply-sparse-index-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1914/derrickstolee/apply-sparse-index-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/1914

Re: [PATCH 0/3] Integrate the sparse index with 'git apply' and 'git add -p/-i'

[Derrick Stolee]

On 5/14/25 11:16 AM, Phillip Wood wrote:

> This made me wonder about the other commands that take "--patch" like checkout 
> and reset. Do you know how well they handle the sparse index? They'll all 
> benefit from the changes to git apply in this series but I was wondering if they 
> need any further changes.
By adding these two tests...

   test_perf_on_all 'test_write_lines y y y | git checkout --patch -'
   test_perf_on_all 'echo >>a && git add a && test_write_lines y | git reset 
--patch'

...we can demonstrate that the changes to 'git apply' are sufficient
to get the improvements we seek for 'git checkout --patch':

Test                                   HEAD~3   HEAD~2
---------------------------------------------------------------
... git checkout --patch - (full-v3)     1.22   1.22  +0.0%
... git checkout --patch - (full-v4)     1.15   1.16  +0.9%
... git checkout --patch - (sparse-v3)   1.37   0.11 -92.0%
... git checkout --patch - (sparse-v4)   1.37   0.11 -92.0%
... git reset --patch (full-v3)          0.82   0.81  -1.2%
... git reset --patch (full-v4)          0.76   0.77  +1.3%
... git reset --patch (sparse-v3)        1.57   0.91 -42.0%
... git reset --patch (sparse-v4)        1.59   0.92 -42.1%

But 'git reset --patch' appears to not be fast _enough_. It turns
out that it has the same issue as cmd_add(). I'll add a patch for
this purpose.

Thanks,
-Stolee

Re: [PATCH 0/3] Integrate the sparse index with 'git apply' and 'git add -p/-i'

[phillip.wood123]

On 16/05/2025 14:28, Derrick Stolee wrote:
> On 5/14/25 11:16 AM, Phillip Wood wrote:
> 
>> This made me wonder about the other commands that take "--patch" like 
>> checkout and reset. Do you know how well they handle the sparse index? 
>> They'll all benefit from the changes to git apply in this series but I 
>> was wondering if they need any further changes.
> By adding these two tests...
> 
>    test_perf_on_all 'test_write_lines y y y | git checkout --patch -'
>    test_perf_on_all 'echo >>a && git add a && test_write_lines y | git 
> reset --patch'
> 
> ...we can demonstrate that the changes to 'git apply' are sufficient
> to get the improvements we seek for 'git checkout --patch':
> 
> Test                                   HEAD~3   HEAD~2
> ---------------------------------------------------------------
> ... git checkout --patch - (full-v3)     1.22   1.22  +0.0%
> ... git checkout --patch - (full-v4)     1.15   1.16  +0.9%
> ... git checkout --patch - (sparse-v3)   1.37   0.11 -92.0%
> ... git checkout --patch - (sparse-v4)   1.37   0.11 -92.0%
> ... git reset --patch (full-v3)          0.82   0.81  -1.2%
> ... git reset --patch (full-v4)          0.76   0.77  +1.3%
> ... git reset --patch (sparse-v3)        1.57   0.91 -42.0%
> ... git reset --patch (sparse-v4)        1.59   0.92 -42.1%
> 
> But 'git reset --patch' appears to not be fast _enough_. It turns
> out that it has the same issue as cmd_add(). I'll add a patch for
> this purpose.

That's great, I don't have anymore comments on v2 of this series.

Thanks

Phillip

[PATCH v2 0/4] Integrate the sparse index with 'git apply' and interactive add, checkout, and reset

[Derrick Stolee via GitGitGadget]

The sparse index helps make some Git commands faster when using
sparse-checkout in cone mode. However, not all code paths are aware that the
index can have non-blob entries, so we are careful about rolling this
feature out gradually. The cost of this rollout is that some commands are
slower with the sparse index as they need to expand a sparse index into a
full index in memory, which requires parsing tree objects to construct the
full path list.

This patch series focuses on the 'git add -p' command, which is slow with
the sparse index for a couple of reasons, handled in the first two patches:

 1. 'git add -p' uses 'git apply' as a subcommand and 'git apply' needs
    integration with the sparse index. Luckily, we just need to add the repo
    setting and appropriate tests to confirm it behaves as expected.
 2. The interactive modes of 'git add' ('-p' and '-i') leave cmd_add()
    before the code that sets the repo setting to allow for a sparse index.
    Patch 2 fixes this and adds appropriate tests to confirm the behavior in
    a sparse-checkout.
 3. The interactive mode of 'git reset' leaves cmd_reset() before the code
    that sets the repo setting to allow for the sparse index.

A third patch adds a performance test to p2000-sparse-operations.sh to
confirm that we are getting the performance improvement we expect:

  Test                                      BASE  PATCH 1      PATCH 2      PATCH 3
  -------------------------------------------------------------------------------------
  2000.118: ... git add -p (full-v3)        0.79  0.79  +0.0%  0.82  +3.8%  0.82  +3.8%
  2000.119: ... git add -p (full-v4)        0.74  0.76  +2.7%  0.74  +0.0%  0.76  +2.7%
  2000.120: ... git add -p (sparse-v3)      1.94  1.28 -34.0%  0.07 -96.4%  0.07 -96.4%
  2000.121: ... git add -p (sparse-v4)      1.93  1.28 -33.7%  0.06 -96.9%  0.06 -96.9%
  2000.122: ... git checkout -p (full-v3)   1.18  1.18  +0.0%  1.18  +0.0%  1.19  +0.8%
  2000.123: ... git checkout -p (full-v4)   1.10  1.12  +1.8%  1.11  +0.9%  1.11  +0.9%
  2000.124: ... git checkout -p (sparse-v3) 1.31  0.11 -91.6%  0.11 -91.6%  0.11 -91.6%
  2000.125: ... git checkout -p (sparse-v4) 1.29  0.11 -91.5%  0.11 -91.5%  0.11 -91.5%
  2000.126: ... git reset -p (full-v3)      0.81  0.80  -1.2%  0.83  +2.5%  0.83  +2.5%
  2000.127: ... git reset -p (full-v4)      0.78  0.77  -1.3%  0.77  -1.3%  0.78  +0.0%
  2000.128: ... git reset -p (sparse-v3)    1.58  0.92 -41.8%  0.91 -42.4%  0.07 -95.6%
  2000.129: ... git reset -p (sparse-v4)    1.58  0.92 -41.8%  0.92 -41.8%  0.07 -95.6%



Updates in v2
=============

Thanks for the careful review from Elijah and the pointer from Phillip, we
have these changes:

 1. The tests no longer have different expansion behaviors for 'git add -p'
    and 'git add -i' due to partially-expanded indexes on disk.
 2. We now test 'git checkout -p' and 'git reset -p'.
 3. 'git reset -p' needed some changes to the builtin (similar to 'git add')
    to be fast.

Thanks, -Stolee

Derrick Stolee (4):
  apply: integrate with the sparse index
  git add: make -p/-i aware of sparse index
  reset: integrate sparse index with --patch
  p2000: add performance test for patch-mode commands

 builtin/add.c                            |   7 +-
 builtin/apply.c                          |   7 +-
 builtin/reset.c                          |   6 +-
 t/perf/p2000-sparse-operations.sh        |   3 +
 t/t1092-sparse-checkout-compatibility.sh | 151 +++++++++++++++++++++++
 5 files changed, 167 insertions(+), 7 deletions(-)


base-commit: 6c0bd1fc70efaf053abe4e57c976afdc72d15377
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1914%2Fderrickstolee%2Fapply-sparse-index-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1914/derrickstolee/apply-sparse-index-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1914

Range-diff vs v1:

 1:  0e6e199cd19 ! 1:  1adf81ecb2c apply: integrate with the sparse index
     @@ t/t1092-sparse-checkout-compatibility.sh: test_expect_success 'sparse-index is n
      +
      +	# Expands when using --index.
      +	ensure_expanded apply --index ../patch-outside &&
     ++
     ++	# Does not when index is partially expanded.
     ++	git -C sparse-index reset --hard &&
     ++	ensure_not_expanded apply --cached ../patch-outside &&
     ++
     ++	# Try again with a reset and collapsed index.
      +	git -C sparse-index reset --hard &&
     ++	git -C sparse-index sparse-checkout reapply &&
      +
     -+	# Does not expand when using --cached.
     -+	ensure_not_expanded apply --cached ../patch-outside
     ++	# Expands when index is collapsed.
     ++	ensure_expanded apply --cached ../patch-outside
      +'
      +
       test_expect_success 'advice.sparseIndexExpanded' '
 2:  63caae87634 ! 2:  0a2752721d0 git add: make -p/-i aware of sparse index
     @@ Commit message
      
          It turns out that control flows out of cmd_add() in the interactive
          cases before the lines that confirm that the builtin is integrated with
     -    the sparse index. We need to move that earlier to ensure it prevents a
     -    full index expansion on read.
     +    the sparse index.
      
     -    Add more test cases that confirm that these interactive add options work
     -    with the sparse index. One interesting aspect here is that the '-i'
     -    option avoids expanding the sparse index when a sparse directory exists
     -    on disk while the '-p' option does hit the ensure_full_index() method.
     -    This leaves some room for improvement, but this case should be atypical
     -    as users should remain within their sparse-checkout.
     +    Moving that integration point earlier in cmd_add() allows 'git add -p'
     +    and 'git add -p' to operate without expanding a sparse index to a full
     +    one.
     +
     +    Add test cases that confirm that these interactive add options work with
     +    the sparse index.
      
          Signed-off-by: Derrick Stolee <stolee@gmail.com>
      
     @@ t/t1092-sparse-checkout-compatibility.sh: test_expect_success 'add, commit, chec
       	init_repos &&
       
      @@ t/t1092-sparse-checkout-compatibility.sh: test_expect_success 'sparse-index is not expanded: git apply' '
     - 	ensure_not_expanded apply --cached ../patch-outside
     + 	ensure_expanded apply --cached ../patch-outside
       '
       
      +test_expect_success 'sparse-index is not expanded: git add -p' '
     @@ t/t1092-sparse-checkout-compatibility.sh: test_expect_success 'sparse-index is n
      +	git -C sparse-index reset &&
      +	ensure_not_expanded add -i <in &&
      +
     ++	# -p does expand when edits are outside sparse checkout.
      +	mkdir -p sparse-index/folder1 &&
      +	echo "new content" >sparse-index/folder1/a &&
     -+
     -+	# -p does expand when edits are outside sparse checkout.
      +	test_write_lines y n y >in &&
      +	ensure_expanded add -p <in &&
      +
     -+	# but -i does not expand.
     -+	git -C sparse-index reset &&
     ++	# Fully reset the index.
     ++	git -C sparse-index reset --hard &&
     ++	git -C sparse-index sparse-checkout reapply &&
     ++
     ++	# -i does expand when edits are outside sparse checkout.
     ++	mkdir -p sparse-index/folder1 &&
     ++	echo "new content" >sparse-index/folder1/a &&
      +	test_write_lines u 2 3 "" q >in &&
     -+	ensure_not_expanded add -i <in
     ++	ensure_expanded add -i <in
      +'
      +
       test_expect_success 'advice.sparseIndexExpanded' '
 -:  ----------- > 3:  d1482a29d8f reset: integrate sparse index with --patch
 3:  7a777281626 ! 4:  a50c57f7628 p2000: add performance test for 'git add -p'
     @@ Metadata
      Author: Derrick Stolee <dstolee@microsoft.com>
      
       ## Commit message ##
     -    p2000: add performance test for 'git add -p'
     +    p2000: add performance test for patch-mode commands
      
     -    The previous two changes contributed performance improvements to 'git
     -    apply' and 'git add -p' when using a sparse index. Add a performance
     -    test to demonstrate this (and to help validate that performance remains
     -    good in the future).
     +    The previous three changes contributed performance improvements to 'git
     +    apply', 'git add -p', and 'git reset -p' when using a sparse index. The
     +    improvement to 'git apply' also improved 'git checkout -p'. Add
     +    performance tests to demonstrate this (and to help validate that
     +    performance remains good in the future).
      
          In the truncated test output below, we see that the full checkout
          performance changes within noise expectations, but the sparse index
     -    cases improve 33% and then 96%.
     -
     -                          HEAD~3     HEAD~2         HEAD~1
     -    ---------------------------------------------------------
     -    2000.118: (full-v3)     0.80   0.84 +5.0%     0.84  +5.0%
     -    2000.119: (full-v4)     0.76   0.79 +3.9%     0.80  +5.3%
     -    2000.120: (sparse-v3)   2.09   1.39 -33.5%    0.07 -96.7%
     -    2000.121: (sparse-v4)   2.09   1.39 -33.5%    0.07 -96.7%
     +    cases improve 33% and then 96% for 'git add -p' and 41% and then 95% for
     +    'git reset -p'. 'git checkout -p' improves immediatley by 91% because it
     +    does not need any change to its builtin.
     +
     +      Test                                    HEAD~4  HEAD~3       HEAD~2       HEAD~1
     +      -------------------------------------------------------------------------------------
     +      2000.118: ... git add -p (full-v3)        0.79  0.79  +0.0%  0.82  +3.8%  0.82  +3.8%
     +      2000.119: ... git add -p (full-v4)        0.74  0.76  +2.7%  0.74  +0.0%  0.76  +2.7%
     +      2000.120: ... git add -p (sparse-v3)      1.94  1.28 -34.0%  0.07 -96.4%  0.07 -96.4%
     +      2000.121: ... git add -p (sparse-v4)      1.93  1.28 -33.7%  0.06 -96.9%  0.06 -96.9%
     +      2000.122: ... git checkout -p (full-v3)   1.18  1.18  +0.0%  1.18  +0.0%  1.19  +0.8%
     +      2000.123: ... git checkout -p (full-v4)   1.10  1.12  +1.8%  1.11  +0.9%  1.11  +0.9%
     +      2000.124: ... git checkout -p (sparse-v3) 1.31  0.11 -91.6%  0.11 -91.6%  0.11 -91.6%
     +      2000.125: ... git checkout -p (sparse-v4) 1.29  0.11 -91.5%  0.11 -91.5%  0.11 -91.5%
     +      2000.126: ... git reset -p (full-v3)      0.81  0.80  -1.2%  0.83  +2.5%  0.83  +2.5%
     +      2000.127: ... git reset -p (full-v4)      0.78  0.77  -1.3%  0.77  -1.3%  0.78  +0.0%
     +      2000.128: ... git reset -p (sparse-v3)    1.58  0.92 -41.8%  0.91 -42.4%  0.07 -95.6%
     +      2000.129: ... git reset -p (sparse-v4)    1.58  0.92 -41.8%  0.92 -41.8%  0.07 -95.6%
      
          It is worth noting that if our test was more involved and had multiple
          hunks to evaluate, then the time spent in 'git apply' would dominate due
     @@ t/perf/p2000-sparse-operations.sh: test_perf_on_all git diff-tree HEAD
       test_perf_on_all "git worktree add ../temp && git worktree remove ../temp"
       test_perf_on_all git check-attr -a -- $SPARSE_CONE/a
      +test_perf_on_all 'echo >>a && test_write_lines y | git add -p'
     ++test_perf_on_all 'test_write_lines y y y | git checkout --patch -'
     ++test_perf_on_all 'echo >>a && git add a && test_write_lines y | git reset --patch'
       
       test_done

-- 
gitgitgadget

[PATCH v2 1/4] apply: integrate with the sparse index

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

The sparse index allows storing directory entries in the index, marked
with the skip-wortkree bit and pointing to a tree object. This may be an
unexpected data shape for some implementation areas, so we are rolling
it out incrementally on a builtin-per-builtin basis.

This change enables the sparse index for 'git apply'. The main
motivation for this change is that 'git apply' is used as a child
process of 'git add -p' and expanding the sparse index for each of those
child processes can lead to significant performance issues.

The good news is that the actual index manipulation code used by 'git
apply' is already integrated with the sparse index, so the only product
change is to mark the builtin as allowing the sparse index so it isn't
inflated on read.

The more involved part of this change is around adding tests that verify
how 'git apply' behaves in a sparse-checkout environment and whether or
not the index expands in certain operations.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 builtin/apply.c                          |  7 +++-
 t/t1092-sparse-checkout-compatibility.sh | 53 ++++++++++++++++++++++++
 2 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index 84f1863d3ac3..a1e20c593d09 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -12,7 +12,7 @@ static const char * const apply_usage[] = {
 int cmd_apply(int argc,
 	      const char **argv,
 	      const char *prefix,
-	      struct repository *repo UNUSED)
+	      struct repository *repo)
 {
 	int force_apply = 0;
 	int options = 0;
@@ -35,6 +35,11 @@ int cmd_apply(int argc,
 				   &state, &force_apply, &options,
 				   apply_usage);
 
+	if (repo) {
+		prepare_repo_settings(repo);
+		repo->settings.command_requires_full_index = 0;
+	}
+
 	if (check_apply_state(&state, force_apply))
 		exit(128);
 
diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh
index f9b448792cb4..83353a7dbab4 100755
--- a/t/t1092-sparse-checkout-compatibility.sh
+++ b/t/t1092-sparse-checkout-compatibility.sh
@@ -1340,6 +1340,30 @@ test_expect_success 'submodule handling' '
 	grep "160000 $(git -C initial-repo rev-parse HEAD) 0	modules/sub" cache
 '
 
+test_expect_success 'git apply functionality' '
+	init_repos &&
+
+	test_all_match git checkout base &&
+
+	git -C full-checkout diff base..merge-right -- deep >patch-in-sparse &&
+	git -C full-checkout diff base..merge-right -- folder2 >patch-outside &&
+
+	# Apply a patch to a file inside the sparse definition
+	test_all_match git apply --index --stat ../patch-in-sparse &&
+	test_all_match git status --porcelain=v2 &&
+
+	# Apply a patch to a file outside the sparse definition
+	test_sparse_match test_must_fail git apply ../patch-outside &&
+	grep "No such file or directory" sparse-checkout-err &&
+
+	# But it works with --index and --cached
+	test_all_match git apply --index --stat ../patch-outside &&
+	test_all_match git status --porcelain=v2 &&
+	test_all_match git reset --hard &&
+	test_all_match git apply --cached --stat ../patch-outside &&
+	test_all_match git status --porcelain=v2
+'
+
 # When working with a sparse index, some commands will need to expand the
 # index to operate properly. If those commands also write the index back
 # to disk, they need to convert the index to sparse before writing.
@@ -2345,6 +2369,35 @@ test_expect_success 'sparse-index is not expanded: check-attr' '
 	ensure_not_expanded check-attr -a --cached -- folder1/a
 '
 
+test_expect_success 'sparse-index is not expanded: git apply' '
+	init_repos &&
+
+	git -C sparse-index checkout base &&
+	git -C full-checkout diff base..merge-right -- deep >patch-in-sparse &&
+	git -C full-checkout diff base..merge-right -- folder2 >patch-outside &&
+
+	# Apply a patch to a file inside the sparse definition
+	ensure_not_expanded apply --index --stat ../patch-in-sparse &&
+
+	# Apply a patch to a file outside the sparse definition
+	# Fails when caring about the worktree.
+	ensure_not_expanded ! apply ../patch-outside &&
+
+	# Expands when using --index.
+	ensure_expanded apply --index ../patch-outside &&
+
+	# Does not when index is partially expanded.
+	git -C sparse-index reset --hard &&
+	ensure_not_expanded apply --cached ../patch-outside &&
+
+	# Try again with a reset and collapsed index.
+	git -C sparse-index reset --hard &&
+	git -C sparse-index sparse-checkout reapply &&
+
+	# Expands when index is collapsed.
+	ensure_expanded apply --cached ../patch-outside
+'
+
 test_expect_success 'advice.sparseIndexExpanded' '
 	init_repos &&
 
-- 
gitgitgadget

[PATCH v2 2/4] git add: make -p/-i aware of sparse index

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

It is slow to expand a sparse index in-memory due to parsing of trees.
We aim to minimize that performance cost when possible. 'git add -p'
uses 'git apply' child processes to modify the index, but still there
are some expansions that occur.

It turns out that control flows out of cmd_add() in the interactive
cases before the lines that confirm that the builtin is integrated with
the sparse index.

Moving that integration point earlier in cmd_add() allows 'git add -p'
and 'git add -p' to operate without expanding a sparse index to a full
one.

Add test cases that confirm that these interactive add options work with
the sparse index.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 builtin/add.c                            |  7 +--
 t/t1092-sparse-checkout-compatibility.sh | 60 ++++++++++++++++++++++++
 2 files changed, 64 insertions(+), 3 deletions(-)

diff --git a/builtin/add.c b/builtin/add.c
index 747511b68bc3..7c292ffdc6c2 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -390,6 +390,10 @@ int cmd_add(int argc,
 
 	argc = parse_options(argc, argv, prefix, builtin_add_options,
 			  builtin_add_usage, PARSE_OPT_KEEP_ARGV0);
+
+	prepare_repo_settings(repo);
+	repo->settings.command_requires_full_index = 0;
+
 	if (patch_interactive)
 		add_interactive = 1;
 	if (add_interactive) {
@@ -426,9 +430,6 @@ int cmd_add(int argc,
 	add_new_files = !take_worktree_changes && !refresh_only && !add_renormalize;
 	require_pathspec = !(take_worktree_changes || (0 < addremove_explicit));
 
-	prepare_repo_settings(repo);
-	repo->settings.command_requires_full_index = 0;
-
 	repo_hold_locked_index(repo, &lock_file, LOCK_DIE_ON_ERROR);
 
 	/*
diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh
index 83353a7dbab4..c419d8b57e84 100755
--- a/t/t1092-sparse-checkout-compatibility.sh
+++ b/t/t1092-sparse-checkout-compatibility.sh
@@ -384,6 +384,38 @@ test_expect_success 'add, commit, checkout' '
 	test_all_match git checkout -
 '
 
+test_expect_success 'git add -p' '
+	init_repos &&
+
+	write_script edit-contents <<-\EOF &&
+	echo text >>$1
+	EOF
+
+	# Does not expand when edits are within sparse checkout.
+	run_on_all ../edit-contents deep/a &&
+	run_on_all ../edit-contents deep/deeper1/a &&
+
+	test_write_lines y n >in &&
+	run_on_all git add -p <in &&
+	test_all_match git status --porcelain=v2 &&
+	test_all_match git reset &&
+
+	test_write_lines u 1 "" q >in &&
+	run_on_all git add -i <in &&
+	test_all_match git status --porcelain=v2 &&
+	test_all_match git reset --hard &&
+
+	run_on_sparse mkdir -p folder1 &&
+	run_on_all ../edit-contents folder1/a &&
+	test_write_lines y n y >in &&
+	run_on_all git add -p <in &&
+	test_sparse_match git status --porcelain=v2 &&
+	test_sparse_match git reset &&
+	test_write_lines u 2 3 "" q >in &&
+	run_on_all git add -i <in &&
+	test_sparse_match git status --porcelain=v2
+'
+
 test_expect_success 'deep changes during checkout' '
 	init_repos &&
 
@@ -2398,6 +2430,34 @@ test_expect_success 'sparse-index is not expanded: git apply' '
 	ensure_expanded apply --cached ../patch-outside
 '
 
+test_expect_success 'sparse-index is not expanded: git add -p' '
+	init_repos &&
+
+	# Does not expand when edits are within sparse checkout.
+	echo "new content" >sparse-index/deep/a &&
+	echo "new content" >sparse-index/deep/deeper1/a &&
+	test_write_lines y n >in &&
+	ensure_not_expanded add -p <in &&
+	git -C sparse-index reset &&
+	ensure_not_expanded add -i <in &&
+
+	# -p does expand when edits are outside sparse checkout.
+	mkdir -p sparse-index/folder1 &&
+	echo "new content" >sparse-index/folder1/a &&
+	test_write_lines y n y >in &&
+	ensure_expanded add -p <in &&
+
+	# Fully reset the index.
+	git -C sparse-index reset --hard &&
+	git -C sparse-index sparse-checkout reapply &&
+
+	# -i does expand when edits are outside sparse checkout.
+	mkdir -p sparse-index/folder1 &&
+	echo "new content" >sparse-index/folder1/a &&
+	test_write_lines u 2 3 "" q >in &&
+	ensure_expanded add -i <in
+'
+
 test_expect_success 'advice.sparseIndexExpanded' '
 	init_repos &&
 
-- 
gitgitgadget

[PATCH v2 3/4] reset: integrate sparse index with --patch

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

Similar to the previous change for 'git add -p', the reset builtin
checked for integration with the sparse index after possibly redirecting
its logic toward the interactive logic. This means that the builtin
would expand the sparse index to a full one upon read.

Move this check earlier within cmd_reset() to improve performance here.

Add tests to guarantee that we are not universally expanding the index.
Add behavior tests to check that we are doing the same operations as a
full index.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 builtin/reset.c                          |  6 ++--
 t/t1092-sparse-checkout-compatibility.sh | 42 ++++++++++++++++++++++--
 2 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/builtin/reset.c b/builtin/reset.c
index 73b4537a9a56..dc50ffc1ac59 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -420,6 +420,9 @@ int cmd_reset(int argc,
 		oidcpy(&oid, &tree->object.oid);
 	}
 
+	prepare_repo_settings(the_repository);
+	the_repository->settings.command_requires_full_index = 0;
+
 	if (patch_mode) {
 		if (reset_type != NONE)
 			die(_("options '%s' and '%s' cannot be used together"), "--patch", "--{hard,mixed,soft}");
@@ -457,9 +460,6 @@ int cmd_reset(int argc,
 	if (intent_to_add && reset_type != MIXED)
 		die(_("the option '%s' requires '%s'"), "-N", "--mixed");
 
-	prepare_repo_settings(the_repository);
-	the_repository->settings.command_requires_full_index = 0;
-
 	if (repo_read_index(the_repository) < 0)
 		die(_("index file corrupt"));
 
diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh
index c419d8b57e84..d8101139b40a 100755
--- a/t/t1092-sparse-checkout-compatibility.sh
+++ b/t/t1092-sparse-checkout-compatibility.sh
@@ -384,7 +384,7 @@ test_expect_success 'add, commit, checkout' '
 	test_all_match git checkout -
 '
 
-test_expect_success 'git add -p' '
+test_expect_success 'git add, checkout, and reset with -p' '
 	init_repos &&
 
 	write_script edit-contents <<-\EOF &&
@@ -398,7 +398,7 @@ test_expect_success 'git add -p' '
 	test_write_lines y n >in &&
 	run_on_all git add -p <in &&
 	test_all_match git status --porcelain=v2 &&
-	test_all_match git reset &&
+	test_all_match git reset -p <in &&
 
 	test_write_lines u 1 "" q >in &&
 	run_on_all git add -i <in &&
@@ -413,6 +413,12 @@ test_expect_success 'git add -p' '
 	test_sparse_match git reset &&
 	test_write_lines u 2 3 "" q >in &&
 	run_on_all git add -i <in &&
+	test_sparse_match git status --porcelain=v2 &&
+
+	run_on_all git add --sparse folder1 &&
+	run_on_all git commit -m "take changes" &&
+	test_write_lines y n y >in &&
+	test_sparse_match git checkout HEAD~1 --patch <in &&
 	test_sparse_match git status --porcelain=v2
 '
 
@@ -2458,6 +2464,38 @@ test_expect_success 'sparse-index is not expanded: git add -p' '
 	ensure_expanded add -i <in
 '
 
+test_expect_success 'sparse-index is not expanded: checkout -p, reset -p' '
+	init_repos &&
+
+	# Does not expand when edits are within sparse checkout.
+	echo "new content" >sparse-index/deep/a &&
+	echo "new content" >sparse-index/deep/deeper1/a &&
+	git -C sparse-index commit -a -m "inside-changes" &&
+
+	test_write_lines y y >in &&
+	ensure_not_expanded checkout HEAD~1 --patch <in &&
+
+	echo "new content" >sparse-index/deep/a &&
+	echo "new content" >sparse-index/deep/deeper1/a &&
+	git -C sparse-index add . &&
+	ensure_not_expanded reset --patch <in &&
+
+	# -p does expand when edits are outside sparse checkout.
+	mkdir -p sparse-index/folder1 &&
+	echo "new content" >sparse-index/folder1/a &&
+	git -C sparse-index add --sparse folder1 &&
+	git -C sparse-index sparse-checkout reapply &&
+	ensure_expanded reset --patch <in &&
+
+	# Fully reset the index.
+	mkdir -p sparse-index/folder1 &&
+	echo "new content" >sparse-index/folder1/a &&
+	git -C sparse-index add --sparse folder1 &&
+	git -C sparse-index commit -m "folder1 change" &&
+	git -C sparse-index sparse-checkout reapply &&
+	ensure_expanded checkout HEAD~1 --patch <in
+'
+
 test_expect_success 'advice.sparseIndexExpanded' '
 	init_repos &&
 
-- 
gitgitgadget

Re: [PATCH v2 3/4] reset: integrate sparse index with --patch

[Elijah Newren]

On Fri, May 16, 2025 at 7:55 AM Derrick Stolee via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Derrick Stolee <stolee@gmail.com>
>
> Similar to the previous change for 'git add -p', the reset builtin
> checked for integration with the sparse index after possibly redirecting
> its logic toward the interactive logic. This means that the builtin
> would expand the sparse index to a full one upon read.
>
> Move this check earlier within cmd_reset() to improve performance here.
>
> Add tests to guarantee that we are not universally expanding the index.
> Add behavior tests to check that we are doing the same operations as a
> full index.
>
> Signed-off-by: Derrick Stolee <stolee@gmail.com>
> ---
>  builtin/reset.c                          |  6 ++--
>  t/t1092-sparse-checkout-compatibility.sh | 42 ++++++++++++++++++++++--
>  2 files changed, 43 insertions(+), 5 deletions(-)
>
> diff --git a/builtin/reset.c b/builtin/reset.c
> index 73b4537a9a56..dc50ffc1ac59 100644
> --- a/builtin/reset.c
> +++ b/builtin/reset.c
> @@ -420,6 +420,9 @@ int cmd_reset(int argc,
>                 oidcpy(&oid, &tree->object.oid);
>         }
>
> +       prepare_repo_settings(the_repository);
> +       the_repository->settings.command_requires_full_index = 0;
> +
>         if (patch_mode) {
>                 if (reset_type != NONE)
>                         die(_("options '%s' and '%s' cannot be used together"), "--patch", "--{hard,mixed,soft}");
> @@ -457,9 +460,6 @@ int cmd_reset(int argc,
>         if (intent_to_add && reset_type != MIXED)
>                 die(_("the option '%s' requires '%s'"), "-N", "--mixed");
>
> -       prepare_repo_settings(the_repository);
> -       the_repository->settings.command_requires_full_index = 0;
> -
>         if (repo_read_index(the_repository) < 0)
>                 die(_("index file corrupt"));
>
> diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh
> index c419d8b57e84..d8101139b40a 100755
> --- a/t/t1092-sparse-checkout-compatibility.sh
> +++ b/t/t1092-sparse-checkout-compatibility.sh
> @@ -384,7 +384,7 @@ test_expect_success 'add, commit, checkout' '
>         test_all_match git checkout -
>  '
>
> -test_expect_success 'git add -p' '
> +test_expect_success 'git add, checkout, and reset with -p' '
>         init_repos &&
>
>         write_script edit-contents <<-\EOF &&
> @@ -398,7 +398,7 @@ test_expect_success 'git add -p' '
>         test_write_lines y n >in &&
>         run_on_all git add -p <in &&
>         test_all_match git status --porcelain=v2 &&
> -       test_all_match git reset &&
> +       test_all_match git reset -p <in &&
>
>         test_write_lines u 1 "" q >in &&
>         run_on_all git add -i <in &&
> @@ -413,6 +413,12 @@ test_expect_success 'git add -p' '
>         test_sparse_match git reset &&
>         test_write_lines u 2 3 "" q >in &&
>         run_on_all git add -i <in &&
> +       test_sparse_match git status --porcelain=v2 &&
> +
> +       run_on_all git add --sparse folder1 &&
> +       run_on_all git commit -m "take changes" &&
> +       test_write_lines y n y >in &&
> +       test_sparse_match git checkout HEAD~1 --patch <in &&
>         test_sparse_match git status --porcelain=v2
>  '
>
> @@ -2458,6 +2464,38 @@ test_expect_success 'sparse-index is not expanded: git add -p' '
>         ensure_expanded add -i <in
>  '
>
> +test_expect_success 'sparse-index is not expanded: checkout -p, reset -p' '
> +       init_repos &&
> +
> +       # Does not expand when edits are within sparse checkout.
> +       echo "new content" >sparse-index/deep/a &&
> +       echo "new content" >sparse-index/deep/deeper1/a &&
> +       git -C sparse-index commit -a -m "inside-changes" &&
> +
> +       test_write_lines y y >in &&
> +       ensure_not_expanded checkout HEAD~1 --patch <in &&
> +
> +       echo "new content" >sparse-index/deep/a &&
> +       echo "new content" >sparse-index/deep/deeper1/a &&
> +       git -C sparse-index add . &&
> +       ensure_not_expanded reset --patch <in &&
> +
> +       # -p does expand when edits are outside sparse checkout.
> +       mkdir -p sparse-index/folder1 &&
> +       echo "new content" >sparse-index/folder1/a &&
> +       git -C sparse-index add --sparse folder1 &&
> +       git -C sparse-index sparse-checkout reapply &&
> +       ensure_expanded reset --patch <in &&
> +
> +       # Fully reset the index.
> +       mkdir -p sparse-index/folder1 &&
> +       echo "new content" >sparse-index/folder1/a &&
> +       git -C sparse-index add --sparse folder1 &&
> +       git -C sparse-index commit -m "folder1 change" &&
> +       git -C sparse-index sparse-checkout reapply &&
> +       ensure_expanded checkout HEAD~1 --patch <in
> +'
> +
>  test_expect_success 'advice.sparseIndexExpanded' '
>         init_repos &&
>
> --
> gitgitgadget

Patch looks good to me.

[PATCH v2 4/4] p2000: add performance test for patch-mode commands

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

The previous three changes contributed performance improvements to 'git
apply', 'git add -p', and 'git reset -p' when using a sparse index. The
improvement to 'git apply' also improved 'git checkout -p'. Add
performance tests to demonstrate this (and to help validate that
performance remains good in the future).

In the truncated test output below, we see that the full checkout
performance changes within noise expectations, but the sparse index
cases improve 33% and then 96% for 'git add -p' and 41% and then 95% for
'git reset -p'. 'git checkout -p' improves immediatley by 91% because it
does not need any change to its builtin.

  Test                                    HEAD~4  HEAD~3       HEAD~2       HEAD~1
  -------------------------------------------------------------------------------------
  2000.118: ... git add -p (full-v3)        0.79  0.79  +0.0%  0.82  +3.8%  0.82  +3.8%
  2000.119: ... git add -p (full-v4)        0.74  0.76  +2.7%  0.74  +0.0%  0.76  +2.7%
  2000.120: ... git add -p (sparse-v3)      1.94  1.28 -34.0%  0.07 -96.4%  0.07 -96.4%
  2000.121: ... git add -p (sparse-v4)      1.93  1.28 -33.7%  0.06 -96.9%  0.06 -96.9%
  2000.122: ... git checkout -p (full-v3)   1.18  1.18  +0.0%  1.18  +0.0%  1.19  +0.8%
  2000.123: ... git checkout -p (full-v4)   1.10  1.12  +1.8%  1.11  +0.9%  1.11  +0.9%
  2000.124: ... git checkout -p (sparse-v3) 1.31  0.11 -91.6%  0.11 -91.6%  0.11 -91.6%
  2000.125: ... git checkout -p (sparse-v4) 1.29  0.11 -91.5%  0.11 -91.5%  0.11 -91.5%
  2000.126: ... git reset -p (full-v3)      0.81  0.80  -1.2%  0.83  +2.5%  0.83  +2.5%
  2000.127: ... git reset -p (full-v4)      0.78  0.77  -1.3%  0.77  -1.3%  0.78  +0.0%
  2000.128: ... git reset -p (sparse-v3)    1.58  0.92 -41.8%  0.91 -42.4%  0.07 -95.6%
  2000.129: ... git reset -p (sparse-v4)    1.58  0.92 -41.8%  0.92 -41.8%  0.07 -95.6%

It is worth noting that if our test was more involved and had multiple
hunks to evaluate, then the time spent in 'git apply' would dominate due
to multiple index loads and writes. As it stands, we need the sparse
index improvement in 'git add -p' itself to confirm this performance
improvement.

Since the change for 'git add -i' is identical, we avoid a second test
case for that similar operation.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 t/perf/p2000-sparse-operations.sh | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/t/perf/p2000-sparse-operations.sh b/t/perf/p2000-sparse-operations.sh
index 39e92b084143..aadf22bc2f0b 100755
--- a/t/perf/p2000-sparse-operations.sh
+++ b/t/perf/p2000-sparse-operations.sh
@@ -135,5 +135,8 @@ test_perf_on_all git diff-tree HEAD
 test_perf_on_all git diff-tree HEAD -- $SPARSE_CONE/a
 test_perf_on_all "git worktree add ../temp && git worktree remove ../temp"
 test_perf_on_all git check-attr -a -- $SPARSE_CONE/a
+test_perf_on_all 'echo >>a && test_write_lines y | git add -p'
+test_perf_on_all 'test_write_lines y y y | git checkout --patch -'
+test_perf_on_all 'echo >>a && git add a && test_write_lines y | git reset --patch'
 
 test_done
-- 
gitgitgadget

Re: [PATCH v2 0/4] Integrate the sparse index with 'git apply' and interactive add, checkout, and reset

[Elijah Newren]

On Fri, May 16, 2025 at 7:55 AM Derrick Stolee via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
[...]
> Updates in v2
> =============
>
> Thanks for the careful review from Elijah and the pointer from Phillip, we
> have these changes:
>
>  1. The tests no longer have different expansion behaviors for 'git add -p'
>     and 'git add -i' due to partially-expanded indexes on disk.
>  2. We now test 'git checkout -p' and 'git reset -p'.
>  3. 'git reset -p' needed some changes to the builtin (similar to 'git add')
>     to be fast.

I haven't looked at the new patch yet, but the updated patches 1 & 2
look much improved (though there's still one problem with the new
commit message, which I comment on in the range-diff below).

However, I think Junio already merged your v1 to next
(https://lore.kernel.org/git/CABPp-BEukTWwsuC7MMR8D5_UAhyw-LgT=DsPKAWeR_ZmVVhjzQ@mail.gmail.com/).
So he'll either have to revert your v1 in next and apply the new
series on top, or you'll need to re-roll as fixes on top of your v1.

>  2:  63caae87634 ! 2:  0a2752721d0 git add: make -p/-i aware of sparse index
>      @@ Commit message
>
>           It turns out that control flows out of cmd_add() in the interactive
>           cases before the lines that confirm that the builtin is integrated with
>      -    the sparse index. We need to move that earlier to ensure it prevents a
>      -    full index expansion on read.
>      +    the sparse index.
>
>      -    Add more test cases that confirm that these interactive add options work
>      -    with the sparse index. One interesting aspect here is that the '-i'
>      -    option avoids expanding the sparse index when a sparse directory exists
>      -    on disk while the '-p' option does hit the ensure_full_index() method.
>      -    This leaves some room for improvement, but this case should be atypical
>      -    as users should remain within their sparse-checkout.
>      +    Moving that integration point earlier in cmd_add() allows 'git add -p'
>      +    and 'git add -p' to operate without expanding a sparse index to a full
>      +    one.

Was the second 'git add -p' meant to be 'git add -i'?

>  -:  ----------- > 3:  d1482a29d8f reset: integrate sparse index with --patch

Other than the one comment above, your changes from the range-diff
look good to me for patches 1 & 2, and the new 4.  I haven't looked at
this new patch 3 yet but wanted to comment on the merged-to-next
issue.

Re: [PATCH v2 0/4] Integrate the sparse index with 'git apply' and interactive add, checkout, and reset

[Derrick Stolee]

On 5/16/2025 11:32 AM, Elijah Newren wrote:
> On Fri, May 16, 2025 at 7:55 AM Derrick Stolee via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
>>

>>      -    Add more test cases that confirm that these interactive add options work
>>      -    with the sparse index. One interesting aspect here is that the '-i'
>>      -    option avoids expanding the sparse index when a sparse directory exists
>>      -    on disk while the '-p' option does hit the ensure_full_index() method.
>>      -    This leaves some room for improvement, but this case should be atypical
>>      -    as users should remain within their sparse-checkout.
>>      +    Moving that integration point earlier in cmd_add() allows 'git add -p'
>>      +    and 'git add -p' to operate without expanding a sparse index to a full
>>      +    one.
> 
> Was the second 'git add -p' meant to be 'git add -i'?

You are right. The second one should be 'git add -i'.

And regarding 'next', I do expect my v1 to be ejected from 'next' while
review continues on this v2. If instead it gets merged to 'master', then
I'll prepare a new series on top that applies the learnings from this
review.

Thanks,
-Stolee

Re: [PATCH v2 0/4] Integrate the sparse index with 'git apply' and interactive add, checkout, and reset

[Junio C Hamano]

Elijah Newren <newren@gmail.com> writes:

> However, I think Junio already merged your v1 to next
> (https://lore.kernel.org/git/CABPp-BEukTWwsuC7MMR8D5_UAhyw-LgT=DsPKAWeR_ZmVVhjzQ@mail.gmail.com/).
> So he'll either have to revert your v1 in next and apply the new
> series on top, or you'll need to re-roll as fixes on top of your v1.

Yup, the earlier one will be reverted when I queue this round.

Thanks, both, for being careful.



>
>>  2:  63caae87634 ! 2:  0a2752721d0 git add: make -p/-i aware of sparse index
>>      @@ Commit message
>>
>>           It turns out that control flows out of cmd_add() in the interactive
>>           cases before the lines that confirm that the builtin is integrated with
>>      -    the sparse index. We need to move that earlier to ensure it prevents a
>>      -    full index expansion on read.
>>      +    the sparse index.
>>
>>      -    Add more test cases that confirm that these interactive add options work
>>      -    with the sparse index. One interesting aspect here is that the '-i'
>>      -    option avoids expanding the sparse index when a sparse directory exists
>>      -    on disk while the '-p' option does hit the ensure_full_index() method.
>>      -    This leaves some room for improvement, but this case should be atypical
>>      -    as users should remain within their sparse-checkout.
>>      +    Moving that integration point earlier in cmd_add() allows 'git add -p'
>>      +    and 'git add -p' to operate without expanding a sparse index to a full
>>      +    one.
>
> Was the second 'git add -p' meant to be 'git add -i'?

Good eyes.

>
>>  -:  ----------- > 3:  d1482a29d8f reset: integrate sparse index with --patch
>
> Other than the one comment above, your changes from the range-diff
> look good to me for patches 1 & 2, and the new 4.  I haven't looked at
> this new patch 3 yet but wanted to comment on the merged-to-next
> issue.