On 5/10/25 12:38 AM, Elijah Newren wrote:
On Tue, May 6, 2025 at 5:55 PM Derrick Stolee via GitGitGadget
<gitgitgadget@xxxxxxxxx> wrote:
From: Derrick Stolee <stolee@xxxxxxxxx>
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
