zed icon indicating copy to clipboard operation
zed copied to clipboard

Optimize file finder subscriptions

Open alygin opened this issue 5 months ago • 5 comments

Optimizes file finder subscriptions — it now only subscribes to worktrees updates instead of all project updates.

Project panel could also be optimized this way, I guess.

Related issues:

  • Fixes #7519

Release Notes:

  • Fix selection resets in the file finder during language server startup.

alygin avatar Feb 24 '24 19:02 alygin

What about worktrees addition/deletion, the new code would not react to that?

Good point. No, it wouldn't. But I don't understand, in which practical cases this will be a problem. Worktrees subscriptions are created on each file finder opening, thus we are not considering any manual operations on worktrees as a source of such problems. Initial project opening? I guess, Zed creates worktrees immediately, and then populates them with entries asynchronously. Are there background processes that add or remove worktrees, and can affect search results?

If yes, what do you think would be the right approach to fix here? We could simply subscribe to all the project updates, but only trigger search refresh when the number of workspaces changed. Or we could add something like on_event(&mut self, event_type, handler) to Project and implement WorktreesChangedEvent. What do you think?

alygin avatar Feb 25 '24 05:02 alygin

I think this could be simplified by subscribing to events on the project, instead of observing it. I believe the file finder only needs to update on the ‘WorktreeUpdatedEntries’ event, but we could also add WorktreeAdded and WorktreeRemoved events for completeness.

maxbrunsfeld avatar Feb 25 '24 07:02 maxbrunsfeld

Are there background processes that add or remove worktrees, and can affect search results?

I'm thinking of the collab mode, when the client has the file finder open, and host does some manipulations with the project worktrees (e.g. drags and drops a file with the name matching the file finder query).

As for the approach, I think Max pointed out the right way — maybe it has to be a hybrid of both notification listening and workspace watching.

SomeoneToIgnore avatar Feb 25 '24 07:02 SomeoneToIgnore

I think this could be simplified by subscribing to events on the project, instead of observing it. I believe the file finder only needs to update on the ‘WorktreeUpdatedEntries’ event, but we could also add WorktreeAdded and WorktreeRemoved events for completeness.

Thanks, it works! I'll try to find a way to add unit tests for this change.

alygin avatar Feb 25 '24 11:02 alygin

I'm thinking of the collab mode, when the client has the file finder open, and host does some manipulations with the project worktrees (e.g. drags and drops a file with the name matching the file finder query).

Oh, I see, thanks. I'll try to test this case somehow.

alygin avatar Feb 25 '24 11:02 alygin

Took me a while to figure out that advance_clock() thing in tests 😅 @SomeoneToIgnore, please review

alygin avatar Feb 28 '24 07:02 alygin

I propose to delete the dependency and merge the whole thing, optionally including the conditional compilation proposal.

SomeoneToIgnore avatar Feb 28 '24 12:02 SomeoneToIgnore

Nice work @alygin.

maxbrunsfeld avatar Feb 28 '24 16:02 maxbrunsfeld

@SomeoneToIgnore, @maxbrunsfeld, thanks for your help with this one!

Do you think the project panel may benefit from similar optimization? I guess there will be more event types to handle, but it still should help in cases of language server update storms.

alygin avatar Feb 28 '24 19:02 alygin

Why not. Project panel close-open toggling now resets its revealed item — might be related to such broad subscription and would be nice to fix 🙂

SomeoneToIgnore avatar Feb 28 '24 20:02 SomeoneToIgnore

Why not.

Here it is: #8846

Project panel close-open toggling now resets its revealed item — might be related to such broad subscription and would be nice to fix 🙂

Unfortunately, it doesn't fix the close-open problem :(

alygin avatar Mar 04 '24 18:03 alygin

Project panel close-open toggling now resets its revealed item — might be related to such broad subscription and would be nice to fix 🙂

Here: #8961

alygin avatar Mar 06 '24 19:03 alygin