load files for the command palette async so it opens faster#10332
Conversation
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR moves zero-state command palette file result construction into an async future and yields while iterating so long file lists can be aborted more responsively. The security pass found no security-specific findings.
Concerns
- The expensive repo snapshot still happens synchronously before the async future is spawned, so opening the palette can still block on repo-content cache population or
git statusin the slow cases this PR targets.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| @@ -127,47 +127,54 @@ impl FileDataSource { | |||
|
|
|||
| let (contents, git_changed_files) = self.contents_with_git_changes(app); | |||
There was a problem hiding this comment.
contents_with_git_changes(app) still runs synchronously before the future is spawned, including repo-content cache population and git status; move that snapshot work off the palette-open path so large repos do not still block before yielding.
kevinchevalier
left a comment
There was a problem hiding this comment.
I'm approving since it fixed the issue for the user, but it's unclear to me which part of this is fixing the issue. Which part was slow before?
It seems like computing the contents is still sync, and then the other work were just a couple of hashmap lookups.
Is it the sort?
|
I suspect it's the heap allocations happening inside the for-loop. There could be 100's of thousands if not millions of files. |
Description
This PR refactors the
FileDataSourceto load the files asynchronously. This avoids blocking the main thread and solves the momentary pause when opening the command palette.Linked Issue
Fixes #10256
Testing
No perceived changes in local testing. On slower machines, the results may not actually appear for a couple of seconds while the data source is still computing the results.
I sent the user a custom built from this branch and they have confirmed it solves the issue
#10256 (comment)
Changelog Entries for Stable
CHANGELOG-IMPROVEMENT: Performance improvement for opening the command palette when in a project with a lot of files.