# git add -p is slow for sparse checkout 4 messages from 2024-12-06 to 2024-12-09. Participants: Manoraj K, Shubham Kanodia. Thread: https://gitlist.dev/t/62605 ## Manoraj K, 2024-12-06 03:46 Subject: git add -p is slow for sparse checkout Message-ID: URL: https://gitlist.dev/e/CACfnds%3D%2B%2BC75pg8ojn5sJzjKzLcgnyPxgR2c6pOPCRmf2rDKvQ%40mail.gmail.com ``` Hi team, I am currently on a sparse checkout repository, and it looks like the `git add—p` command is much slower than in a full checkout repository. When run, the `git add—p` command expands to the full index and takes more time to start showing the patch. Also, we have git trace2 enabled, and the timings of the subcommands gave us an indication that the `git apply—-cached` subcommand is taking way too long, with an average P50 of 3000ms against 300ms in the full checkout repository. The other notable spikes are in `git diff-files --color` (180ms vs 80ms) and `git diff-files --no-color` (180ms vs 80ms). I am really interested in understanding why `git add -p` expands to a full index, which I believe is the issue with start-up regression. Also, would be great to understand the difference in performance of the subcommands. ``` ## Manoraj K, 2024-12-06 04:23 Subject: Re: git add -p is slow for sparse checkout Message-ID: URL: https://gitlist.dev/e/CACfnds%3DJ%3DQ%2BviagTEoiQ3edUxU8GSY_s7%3DbiGUGY4NKbQpsTvw%40mail.gmail.com In-Reply-To: ``` ++ CC On Fri, Dec 6, 2024 at 9:16 AM Manoraj K wrote: > > Hi team, > > I am currently on a sparse checkout repository, and it looks like the > `git add—p` command is much slower than in a full checkout repository. > > When run, the `git add—p` command expands to the full index and takes > more time to start showing the patch. > > Also, we have git trace2 enabled, and the timings of the subcommands > gave us an indication that the `git apply—-cached` subcommand is > taking way too long, with an average P50 of 3000ms against 300ms in > the full checkout repository. The other notable spikes are in `git > diff-files --color` (180ms vs 80ms) and `git diff-files --no-color` > (180ms vs 80ms). > > I am really interested in understanding why `git add -p` expands to a > full index, which I believe is the issue with start-up regression. > Also, would be great to understand the difference in performance of > the subcommands. ``` ## Shubham Kanodia, 2024-12-06 08:43 Subject: Re: git add -p is slow for sparse checkout Message-ID: URL: https://gitlist.dev/e/CAARMkVGYOKb0H3pmUdcDWTs2p3hZZMX0dsdU1rT%3DHg5a6M7Z6g%40mail.gmail.com In-Reply-To: ``` On Fri, Dec 6, 2024 at 9:53 AM Manoraj K wrote: > > ++ CC > > On Fri, Dec 6, 2024 at 9:16 AM Manoraj K wrote: > > > > Hi team, > > > > I am currently on a sparse checkout repository, and it looks like the > > `git add—p` command is much slower than in a full checkout repository. > > > > When run, the `git add—p` command expands to the full index and takes > > more time to start showing the patch. > > > > Also, we have git trace2 enabled, and the timings of the subcommands > > gave us an indication that the `git apply—-cached` subcommand is > > taking way too long, with an average P50 of 3000ms against 300ms in > > the full checkout repository. The other notable spikes are in `git > > diff-files --color` (180ms vs 80ms) and `git diff-files --no-color` > > (180ms vs 80ms). > > > > I am really interested in understanding why `git add -p` expands to a > > full index, which I believe is the issue with start-up regression. > > Also, would be great to understand the difference in performance of > > the subcommands. I started tracing execution of `git add -p`, it seems that `git add -p` seems to require a full index whereas a regular `git add` does not, because `git add -p` ends up calling `do_read_index` in `read_cache.c`. `do_read_index` seems to then call `prepare_repo_settings` that resets the value of `istate->repo.settings.command_requires_full_index` to `1`, even though `istate->repo.settings.command_requires_full_index` passed to `do_read_index` was initially `0` for `git add -p`. ˋˋˋ int do_read_index(struct index_state *istate, const char *path, int must_exist) { .... /* * If the command explicitly requires a full index, force it * to be full. Otherwise, correct the sparsity based on repository * settings and other properties of the index (if necessary). */ prepare_repo_settings(istate->repo); ˋˋˋ The comment here seems confusing to me — it seems like the intent here was to preserve sparseness when as possible, but `prepare_repo_settings` seemsto do a blanket reset — ˋˋˋ void prepare_repo_settings(struct repository *r) { .... /* * This setting guards all index reads to require a full index * over a sparse index. After suitable guards are placed in the * codebase around uses of the index, this setting will be * removed. */ r->settings.command_requires_full_index = 1; ˋˋˋ Retaining the original setting seems to fix the problem and makes `git add -p` fast, however I don't know if there's a reason why it wasn't already done this way. ˋˋˋ read-cache.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/read-cache.c b/read-cache.c index 01d0b3ad22..92f7561a88 100644 --- a/read-cache.c +++ b/read-cache.c @@ -2339,7 +2339,12 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist) * to be full. Otherwise, correct the sparsity based on repository * settings and other properties of the index (if necessary). */ + int previous_full_index_setting = + istate->repo->settings.command_requires_full_index; + prepare_repo_settings(istate->repo); + istate->repo->settings.command_requires_full_index = previous_full_index_setting; + if (istate->repo->settings.command_requires_full_index) ensure_full_index(istate); else ˋˋˋ ``` ## Shubham Kanodia, 2024-12-09 08:18 Subject: Re: git add -p is slow for sparse checkout Message-ID: URL: https://gitlist.dev/e/CAARMkVGxduJZnxBAK9smL4JnBcUZ44tOsH7m-WgSa3ymtATt1w%40mail.gmail.com In-Reply-To: ``` On Fri, Dec 6, 2024 at 2:13 PM Shubham Kanodia wrote: > > On Fri, Dec 6, 2024 at 9:53 AM Manoraj K wrote: > > > > ++ CC > > > > On Fri, Dec 6, 2024 at 9:16 AM Manoraj K wrote: > > > > > > Hi team, > > > > > > I am currently on a sparse checkout repository, and it looks like the > > > `git add—p` command is much slower than in a full checkout repository. > > > > > > When run, the `git add—p` command expands to the full index and takes > > > more time to start showing the patch. > > > > > > Also, we have git trace2 enabled, and the timings of the subcommands > > > gave us an indication that the `git apply—-cached` subcommand is > > > taking way too long, with an average P50 of 3000ms against 300ms in > > > the full checkout repository. The other notable spikes are in `git > > > diff-files --color` (180ms vs 80ms) and `git diff-files --no-color` > > > (180ms vs 80ms). > > > > > > I am really interested in understanding why `git add -p` expands to a > > > full index, which I believe is the issue with start-up regression. > > > Also, would be great to understand the difference in performance of > > > the subcommands. > > I started tracing execution of `git add -p`, it seems that `git add > -p` seems to require a > full index whereas a regular `git add` does not, because `git add -p` > ends up calling > `do_read_index` in `read_cache.c`. > > > `do_read_index` seems to then call `prepare_repo_settings` that resets > the value of > `istate->repo.settings.command_requires_full_index` to `1`, even though > `istate->repo.settings.command_requires_full_index` passed to > `do_read_index` was > initially `0` for `git add -p`. > > ˋˋˋ > int do_read_index(struct index_state *istate, const char *path, int must_exist) > { > .... > /* > * If the command explicitly requires a full index, force it > * to be full. Otherwise, correct the sparsity based on repository > * settings and other properties of the index (if necessary). > */ > prepare_repo_settings(istate->repo); > ˋˋˋ > > The comment here seems confusing to me — it seems like the intent here > was to preserve > sparseness when as possible, but `prepare_repo_settings` seemsto do a > blanket reset — > > ˋˋˋ > void prepare_repo_settings(struct repository *r) > { > .... > /* > * This setting guards all index reads to require a full index > * over a sparse index. After suitable guards are placed in the > * codebase around uses of the index, this setting will be > * removed. > */ > r->settings.command_requires_full_index = 1; > ˋˋˋ > > Retaining the original setting seems to fix the problem and makes `git > add -p` fast, > however I don't know if there's a reason why it wasn't already done this way. > > ˋˋˋ > read-cache.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/read-cache.c b/read-cache.c > index 01d0b3ad22..92f7561a88 100644 > --- a/read-cache.c > +++ b/read-cache.c > @@ -2339,7 +2339,12 @@ int do_read_index(struct index_state *istate, > const char *path, int must_exist) > * to be full. Otherwise, correct the sparsity based on repository > * settings and other properties of the index (if necessary). > */ > + int previous_full_index_setting = > + istate->repo->settings.command_requires_full_index; > + > prepare_repo_settings(istate->repo); > + istate->repo->settings.command_requires_full_index = > previous_full_index_setting; > + > if (istate->repo->settings.command_requires_full_index) > ensure_full_index(istate); > else > ˋˋˋ Adding Derick — in case you know if indeed a non-sparse index is necessary for `git add -p` or if this could just have been an oversight given support for sparse checkouts was added progressively to git. -- Regards, Shubham Kanodia ```