Re: [RFC PATCH 5/5] Accelerate ensure_skip_worktree_means_skip_worktree by caching

From: Elijah Newren <newren@gmail.com>
Date: Jan 11, 2022, 22:04 UTC
Message-ID: <CABPp-BHEuLB9u1ggDUsTZ_85S52XScM4znM7dnrQQ1iKnXxmhw@mail.gmail.com>
In-Reply-To: <950da537-941f-dfb4-460a-1aa4c68e0750@github.com>

On Tue, Jan 11, 2022 at 10:30 AM Victoria Dye <vdye@github.com> wrote:

Show 23 quoted lines

>
> Elijah Newren wrote:
> > Rather than lstat()'ing every SKIP_WORKTREE path, take advantage of the
> > fact that entire directories will often be missing, especially for cone
> > mode and even more so ever since commit 55dfcf9591 ("sparse-checkout:
> > clear tracked sparse dirs", 2021-09-08).  If we have already determined
> > that the parent directory of a file (or any other previous ancestor)
> > does not exist, then we already know the file cannot exist and do not
> > need to lstat() it separately.
> >
> > Granted, the cost of ensure_skip_worktree_means_skip_worktree() might
> > be considered a bit high for non-cone mode since it might now lstat()
> > every SKIP_WORKTREE path when the index is loaded (an O(N) cost, with
> > N the number of SKIP_WORKTREE paths), but non-cone mode users already
> > have to deal with the O(N*M) cost (with N=the number of tracked files
> > and M=the number of sparsity patterns), so this should be reasonable.
> >
>
> Did you write/run any performance tests to see how this optimization changed
> the execution time? If not, running the `p2000` performance tests against
> the patch series base, [3/5], and [5/5] would provide some really helpful
> insight into the cost of `ensure_skip_worktree_means_skip_worktree`, then
> how much this optimization improves it.

I haven't[1].  You bring up a very good point; I'll add it for the next round.

[1] Long, probably irrelevant story about why: My original patches were actually going to go further and just remove the present-despite-SKIP_WORKTREE files in _all_ cases, sparse-checkout or not. It had not occurred to me while writing the patches to make it specific to sparse-checkouts. Because of that, I figured it was better to get feedback on if the idea was acceptable and spent a lot more time concentrating on making the case. Then I realized near the end that folks who don't use sparse-checkout or SKIP_WORKTREE might be annoyed at the overhead also being added for them, for a feature they'll never even use. I decided to back off a bit, and make it sparse-checkout specific. Then I realized that backing off might just keep all users happy anyway (the folks who intentionally use present-despite-SKIP_WORKTREE paths, despite their many warts, can keep doings so) and edited a lot of my commit messages, docs, and cover letter. And by then it was late Saturday night and I had promised to send out my series on Friday. Since I had already marked my cover letter as RFC anyway, I just decided to temporarily punt on getting performance numbers...