* [PATCH] wt-status: avoid quadratic insertion for untracked paths
@ 2026-07-16 18:50 Sahitya Chandra
2026-07-17 6:27 ` Patrick Steinhardt
2026-07-17 14:46 ` [PATCH v2] wt-status: avoid repeated " Sahitya Chandra
0 siblings, 2 replies; 6+ messages in thread
From: Sahitya Chandra @ 2026-07-16 18:50 UTC (permalink / raw)
To: git; +Cc: gitster, avarab, stolee, peff, Sahitya Chandra
wt_status_collect_untracked() copies entries from dir.entries and
dir.ignored into string_lists using string_list_insert(). That keeps the
destination lists sorted and deduplicated, but each insertion may shift
the backing array, making construction O(n^2) in the number of paths.
Collect the entries with string_list_append() instead, then sort and
deduplicate each list once. This preserves the sorted, duplicate-free
result while reducing the construction cost to O(n log n).
Signed-off-by: Sahitya Chandra <sahityajb@gmail.com>
---
Notes for reviewers:
fill_directory() currently sorts dir.entries and dir.ignored
before returning, so another possible approach would be to append the
entries directly and rely on that order, reducing this copy step to O(n).
That would require relying on these arrays not containing duplicate
entries, though, which I have not been able to verify yet. This patch
takes the safer approach of preserving the existing duplicate-removal
behavior from `string_list_insert()` by sorting and deduplicating once
after appending.
wt-status.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/wt-status.c b/wt-status.c
index 58461e02f8..13a7cf7946 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -832,14 +832,18 @@ static void wt_status_collect_untracked(struct wt_status *s)
for (i = 0; i < dir.nr; i++) {
struct dir_entry *ent = dir.entries[i];
if (index_name_is_other(istate, ent->name, ent->len))
- string_list_insert(&s->untracked, ent->name);
+ string_list_append(&s->untracked, ent->name);
}
+ string_list_sort(&s->untracked);
+ string_list_remove_duplicates(&s->untracked, 0);
for (i = 0; i < dir.ignored_nr; i++) {
struct dir_entry *ent = dir.ignored[i];
if (index_name_is_other(istate, ent->name, ent->len))
- string_list_insert(&s->ignored, ent->name);
+ string_list_append(&s->ignored, ent->name);
}
+ string_list_sort(&s->ignored);
+ string_list_remove_duplicates(&s->ignored, 0);
dir_clear(&dir);
base-commit: d35c5399e3e54ac277bb391fc2f6be3e816d312b
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] wt-status: avoid quadratic insertion for untracked paths
2026-07-16 18:50 [PATCH] wt-status: avoid quadratic insertion for untracked paths Sahitya Chandra
@ 2026-07-17 6:27 ` Patrick Steinhardt
2026-07-17 7:54 ` Jeff King
2026-07-17 14:37 ` Sahitya Chandra
2026-07-17 14:46 ` [PATCH v2] wt-status: avoid repeated " Sahitya Chandra
1 sibling, 2 replies; 6+ messages in thread
From: Patrick Steinhardt @ 2026-07-17 6:27 UTC (permalink / raw)
To: Sahitya Chandra; +Cc: git, gitster, avarab, stolee, peff
On Fri, Jul 17, 2026 at 12:20:45AM +0530, Sahitya Chandra wrote:
> wt_status_collect_untracked() copies entries from dir.entries and
> dir.ignored into string_lists using string_list_insert(). That keeps the
> destination lists sorted and deduplicated, but each insertion may shift
> the backing array, making construction O(n^2) in the number of paths.
>
> Collect the entries with string_list_append() instead, then sort and
> deduplicate each list once. This preserves the sorted, duplicate-free
> result while reducing the construction cost to O(n log n).
>
> Signed-off-by: Sahitya Chandra <sahityajb@gmail.com>
> ---
> Notes for reviewers:
>
> fill_directory() currently sorts dir.entries and dir.ignored
> before returning, so another possible approach would be to append the
> entries directly and rely on that order, reducing this copy step to O(n).
> That would require relying on these arrays not containing duplicate
> entries, though, which I have not been able to verify yet. This patch
> takes the safer approach of preserving the existing duplicate-removal
> behavior from `string_list_insert()` by sorting and deduplicating once
> after appending.
Out of curiosity: is this something that you have encountered in the
real world as inefficient, or is this rather a theoretical inefficiency?
If the former it would be great to add a small benchmark to the commit
message.
> diff --git a/wt-status.c b/wt-status.c
> index 58461e02f8..13a7cf7946 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -832,14 +832,18 @@ static void wt_status_collect_untracked(struct wt_status *s)
> for (i = 0; i < dir.nr; i++) {
> struct dir_entry *ent = dir.entries[i];
> if (index_name_is_other(istate, ent->name, ent->len))
> - string_list_insert(&s->untracked, ent->name);
> + string_list_append(&s->untracked, ent->name);
> }
> + string_list_sort(&s->untracked);
> + string_list_remove_duplicates(&s->untracked, 0);
Instead of sorting and then deduplicating you can call
`string_list_sort_u()`. It does the exact same thing as you do here, but
I guess it makes sense to use that interface anyway.
> for (i = 0; i < dir.ignored_nr; i++) {
> struct dir_entry *ent = dir.ignored[i];
> if (index_name_is_other(istate, ent->name, ent->len))
> - string_list_insert(&s->ignored, ent->name);
> + string_list_append(&s->ignored, ent->name);
> }
> + string_list_sort(&s->ignored);
> + string_list_remove_duplicates(&s->ignored, 0);
Likewise.
Overall this looks like a sensible thing to do though. Thanks!
Patrick
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] wt-status: avoid quadratic insertion for untracked paths
2026-07-17 6:27 ` Patrick Steinhardt
@ 2026-07-17 7:54 ` Jeff King
2026-07-17 14:40 ` Sahitya Chandra
2026-07-17 14:37 ` Sahitya Chandra
1 sibling, 1 reply; 6+ messages in thread
From: Jeff King @ 2026-07-17 7:54 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: Sahitya Chandra, git, gitster, avarab, stolee
On Fri, Jul 17, 2026 at 08:27:09AM +0200, Patrick Steinhardt wrote:
> > fill_directory() currently sorts dir.entries and dir.ignored
> > before returning, so another possible approach would be to append the
> > entries directly and rely on that order, reducing this copy step to O(n).
> > That would require relying on these arrays not containing duplicate
> > entries, though, which I have not been able to verify yet. This patch
> > takes the safer approach of preserving the existing duplicate-removal
> > behavior from `string_list_insert()` by sorting and deduplicating once
> > after appending.
>
> Out of curiosity: is this something that you have encountered in the
> real world as inefficient, or is this rather a theoretical inefficiency?
> If the former it would be great to add a small benchmark to the commit
> message.
Yeah, I had the same question, and tried for a moment to produce an
example before realizing that it probably is theoretical. If we are
feeding the entries in pre-sorted order then the insert is always O(1).
I think it's still worth doing this, though, as it makes the result much
more obvious to analyze. I think it could even be O(n) if the sort
implementation is optimized under the hood for pre-sorted inputs.
-Peff
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] wt-status: avoid quadratic insertion for untracked paths
2026-07-17 6:27 ` Patrick Steinhardt
2026-07-17 7:54 ` Jeff King
@ 2026-07-17 14:37 ` Sahitya Chandra
1 sibling, 0 replies; 6+ messages in thread
From: Sahitya Chandra @ 2026-07-17 14:37 UTC (permalink / raw)
To: ps; +Cc: git, gitster, avarab, stolee, peff
On Fri, Jul 17, 2026 at 11:57 AM Patrick Steinhardt <ps@pks.im> wrote:
> Out of curiosity: is this something that you have encountered in the
> real world as inefficient, or is this rather a theoretical inefficiency?
> If the former it would be great to add a small benchmark to the commit
> message.
Thanks for asking. I do not have a real-world benchmark for this. After
Jeff's reply, I agree that the O(n^2) claim is too strong for the current
code path because fill_directory() already returns the entries sorted, so
string_list_insert() should usually append at the end.
I have reworded v2 to avoid that performance claim and describe the
change as making the collection strategy explicit instead.
> Instead of sorting and then deduplicating you can call
> `string_list_sort_u()`. It does the exact same thing as you do here, but
> I guess it makes sense to use that interface anyway.
Done in v2. Thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] wt-status: avoid quadratic insertion for untracked paths
2026-07-17 7:54 ` Jeff King
@ 2026-07-17 14:40 ` Sahitya Chandra
0 siblings, 0 replies; 6+ messages in thread
From: Sahitya Chandra @ 2026-07-17 14:40 UTC (permalink / raw)
To: Jeff King; +Cc: Patrick Steinhardt, git, gitster, avarab, stolee
On Fri, Jul 17, 2026 at 1:24 PM Jeff King <peff@peff.net> wrote:
> Yeah, I had the same question, and tried for a moment to produce an
> example before realizing that it probably is theoretical. If we are
> feeding the entries in pre-sorted order then the insert is always O(1).
>
> I think it's still worth doing this, though, as it makes the result much
> more obvious to analyze. I think it could even be O(n) if the sort
> implementation is optimized under the hood for pre-sorted inputs.
Thanks, that makes sense. I updated v2 to avoid claiming this is a
current O(n^2) problem and instead frame it as making the append, sort,
and deduplicate steps explicit.
I also switched to string_list_sort_u() as Patrick suggested.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] wt-status: avoid repeated insertion for untracked paths
2026-07-16 18:50 [PATCH] wt-status: avoid quadratic insertion for untracked paths Sahitya Chandra
2026-07-17 6:27 ` Patrick Steinhardt
@ 2026-07-17 14:46 ` Sahitya Chandra
1 sibling, 0 replies; 6+ messages in thread
From: Sahitya Chandra @ 2026-07-17 14:46 UTC (permalink / raw)
To: git; +Cc: gitster, avarab, stolee, peff, ps, Sahitya Chandra
wt_status_collect_untracked() copies entries from dir.entries and
dir.ignored into string_lists using string_list_insert(). That keeps the
destination lists sorted and deduplicated, but makes the code harder to
reason about because it rebuilds sorted lists through repeated sorted
insertion.
Collect the entries with string_list_append() instead, then sort and
deduplicate each list once with string_list_sort_u(). This preserves the
sorted, duplicate-free result while making the collection strategy explicit.
Signed-off-by: Sahitya Chandra <sahityajb@gmail.com>
---
Changes since v1:
- Use string_list_sort_u() instead of open-coding sort plus deduplication.
- Reword the subject and commit message to avoid overclaiming an O(n^2)
cost when the input from fill_directory() is already sorted.
wt-status.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/wt-status.c b/wt-status.c
index 58461e02f8..57772c7501 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -832,14 +832,16 @@ static void wt_status_collect_untracked(struct wt_status *s)
for (i = 0; i < dir.nr; i++) {
struct dir_entry *ent = dir.entries[i];
if (index_name_is_other(istate, ent->name, ent->len))
- string_list_insert(&s->untracked, ent->name);
+ string_list_append(&s->untracked, ent->name);
}
+ string_list_sort_u(&s->untracked, 0);
for (i = 0; i < dir.ignored_nr; i++) {
struct dir_entry *ent = dir.ignored[i];
if (index_name_is_other(istate, ent->name, ent->len))
- string_list_insert(&s->ignored, ent->name);
+ string_list_append(&s->ignored, ent->name);
}
+ string_list_sort_u(&s->ignored, 0);
dir_clear(&dir);
base-commit: 44de1520f08d1dfebc3ab2d9f644208eaa5ac925
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-07-17 14:46 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-16 18:50 [PATCH] wt-status: avoid quadratic insertion for untracked paths Sahitya Chandra
2026-07-17 6:27 ` Patrick Steinhardt
2026-07-17 7:54 ` Jeff King
2026-07-17 14:40 ` Sahitya Chandra
2026-07-17 14:37 ` Sahitya Chandra
2026-07-17 14:46 ` [PATCH v2] wt-status: avoid repeated " Sahitya Chandra
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox