Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Zed is continually writing to disk while idling #16472

Closed
1 task done
marvin-j97 opened this issue Aug 19, 2024 · 9 comments
Closed
1 task done

Zed is continually writing to disk while idling #16472

marvin-j97 opened this issue Aug 19, 2024 · 9 comments
Labels
bug [core label] linux linux-x11 Linux X11 performance Feedback for performance issues, speed, memory usage, etc

Comments

@marvin-j97
Copy link

Check for existing issues

  • Completed

Describe the bug / provide steps to reproduce it

Zed is continually writing to disk (about 200KB per second) even while idling, in any project, even empty projects.

See the gif for comparison with VS Code, which shows it's writing 0 while idling (right most column).

Environment

Zed: v0.148.1 (Zed)
OS: Linux X11 ubuntu 22.04
Memory: 31.2 GiB
Architecture: x86_64
GPU: NVIDIA GeForce RTX 2080 Ti || NVIDIA || 545.29.06

If applicable, add mockups / screenshots to help explain present your vision of the feature

Peek 2024-08-19 17-38

If applicable, attach your Zed.log file to this issue.

Logs are not showing anything after startup

@marvin-j97 marvin-j97 added admin read Pending admin review bug [core label] triage Maintainer needs to classify the issue labels Aug 19, 2024
@JosephTLyons JosephTLyons added performance Feedback for performance issues, speed, memory usage, etc linux linux-x11 Linux X11 and removed triage Maintainer needs to classify the issue admin read Pending admin review labels Aug 23, 2024
@rohitshriwas
Copy link

I can confirm this behaviour on Zed v0.152.4 on Linux (Flatpak). Is this bad for SSD?

image

@github-actions github-actions bot added admin read Pending admin review triage Maintainer needs to classify the issue labels Nov 5, 2024
@notpeter notpeter removed triage Maintainer needs to classify the issue admin read Pending admin review labels Nov 5, 2024
@pepyakin
Copy link
Contributor

pepyakin commented Jan 3, 2025

I was flying on a plane and realized that my battery is drained if not faster but definitely on par with vscode. I decided to take a look and realized the it's the excessive disk writes.

The writes are coming from sqlite. They are mostly invoked WorkspaceDb.

If you look where Workspace::serialize_workspace is invoked it's basically coming from cx.observe on dock views. What's funny is that it is triggered from -[NSWindow _handleMouseDraggedEvent:]. So each time you drag a mouse around the elements it ultimatelly triggers a sqlite write. To add salt to the wound, all the sqlite statements are NOT prepared.

Oh, and this is on macOS, so I don't think the labels are correct (cc @JosephTLyons)

@kingsidelee
Copy link

you can workaround this by open up zed using envvar ZED_STATELESS=1, it will force the program to use a in-memory sqlite db.

zed/crates/db/src/db.rs

Lines 49 to 51 in e4eef72

if *ZED_STATELESS {
return open_fallback_db().await;
}

this won't solve the performance issue, it will still waste some cpu cycles to write the in-memory db.

github-merge-queue bot pushed a commit that referenced this issue Jan 7, 2025
Part of #16472

Reduces amount of workspace serialization happening by:
* fixing the deserialization logic: now it does not set panels that are
hidden to active
* cleaning up `active_panel_index` for docks that are closed, to avoid
emitting extra events for such closed docks
* adjusting outline panel to drop active editor subscriptions on
deactivation — this way, `cx.observe` on the dock with outline panel is
not triggered (used to be triggered on every selection change before)
* adjusting workspace dock drag listener to remember previous
coordinates and only resize the dock if those had changed
* adjusting workspace pane event listener to ignore
`pane::Event::UserSavedItem` and `pane::Event::ChangeItemTitle` that
seem to happen relatively frequently but not influence values that are
serialized for the workspace
* not using `cx.observe` on docks, instead explicitly serializing on
panel zoom and size changes

Release Notes:

- Reduced amount of workspace serialization happening
@SomeoneToIgnore
Copy link
Contributor

Thank you for the great observe find, I've merged #22730 to fix that.
It gets released tomorrow, and, on my tests, removes most of the "extra" writes to rerialize the workspace — would be really interested to see if anything's left to fix after your testing.

gcp-cherry-pick-bot bot pushed a commit that referenced this issue Jan 7, 2025
Part of #16472

Reduces amount of workspace serialization happening by:
* fixing the deserialization logic: now it does not set panels that are
hidden to active
* cleaning up `active_panel_index` for docks that are closed, to avoid
emitting extra events for such closed docks
* adjusting outline panel to drop active editor subscriptions on
deactivation — this way, `cx.observe` on the dock with outline panel is
not triggered (used to be triggered on every selection change before)
* adjusting workspace dock drag listener to remember previous
coordinates and only resize the dock if those had changed
* adjusting workspace pane event listener to ignore
`pane::Event::UserSavedItem` and `pane::Event::ChangeItemTitle` that
seem to happen relatively frequently but not influence values that are
serialized for the workspace
* not using `cx.observe` on docks, instead explicitly serializing on
panel zoom and size changes

Release Notes:

- Reduced amount of workspace serialization happening
SomeoneToIgnore added a commit that referenced this issue Jan 7, 2025
Part of #16472

Reduces amount of workspace serialization happening by:
* fixing the deserialization logic: now it does not set panels that are
hidden to active
* cleaning up `active_panel_index` for docks that are closed, to avoid
emitting extra events for such closed docks
* adjusting outline panel to drop active editor subscriptions on
deactivation — this way, `cx.observe` on the dock with outline panel is
not triggered (used to be triggered on every selection change before)
* adjusting workspace dock drag listener to remember previous
coordinates and only resize the dock if those had changed
* adjusting workspace pane event listener to ignore
`pane::Event::UserSavedItem` and `pane::Event::ChangeItemTitle` that
seem to happen relatively frequently but not influence values that are
serialized for the workspace
* not using `cx.observe` on docks, instead explicitly serializing on
panel zoom and size changes

Release Notes:

- Reduced amount of workspace serialization happening
SomeoneToIgnore added a commit that referenced this issue Jan 7, 2025
… (#22763)

Cherry-picked Reduce amount of workspace serialization happening
(#22730)

Part of #16472

Reduces amount of workspace serialization happening by:
* fixing the deserialization logic: now it does not set panels that are
hidden to active
* cleaning up `active_panel_index` for docks that are closed, to avoid
emitting extra events for such closed docks
* adjusting outline panel to drop active editor subscriptions on
deactivation — this way, `cx.observe` on the dock with outline panel is
not triggered (used to be triggered on every selection change before)
* adjusting workspace dock drag listener to remember previous
coordinates and only resize the dock if those had changed
* adjusting workspace pane event listener to ignore
`pane::Event::UserSavedItem` and `pane::Event::ChangeItemTitle` that
seem to happen relatively frequently but not influence values that are
serialized for the workspace
* not using `cx.observe` on docks, instead explicitly serializing on
panel zoom and size changes

Release Notes:

- Reduced amount of workspace serialization happening

Co-authored-by: Kirill Bulatov <kirill@zed.dev>
@pepyakin
Copy link
Contributor

pepyakin commented Jan 7, 2025

@SomeoneToIgnore can confirm it makes a significant improvement!

That being said, there is some work to do. For example, when you click on an item in the project panel, it still does like 5 calls to serialize_workspace. Can we debounce it by increasing the delay up to 1s?

@SomeoneToIgnore
Copy link
Contributor

Yes, this is closer to a stub than a full bulletproof fix, alas.

I wanted to bump the serialization debounce, but #22730 (comment) was voiced against that so it's not the way we're following it seems.

Still, even with the current value, for a patch

diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs
index e9676dd4a0..92b10b4d61 100644
--- a/crates/workspace/src/workspace.rs
+++ b/crates/workspace/src/workspace.rs
@@ -4130,6 +4130,7 @@ impl Workspace {
         let Some(database_id) = self.database_id() else {
             return Task::ready(());
         };
+        dbg!("serializing the workspace");

         fn serialize_pane_handle(pane_handle: &View<Pane>, cx: &WindowContext) -> SerializedPane {
             let (items, active, pinned_count) = {

that's in the serialization start, I see only one actual serialization when clicking the project panel items:

inevitable.mov

Which seems to work as expected, given that workspace has to remember which item to show on restore.
Do you see a different picture with this patch or is it the DB writes themselves that you see different?
Curious what's not matching in our set-ups.

I have tested all the panels before merging this, and all of them seem to work in a similar way (1 or 0 serialization attempts) except the outline panel which is changing the active item and cause more serialization than needed.
That bit I plan to patch later, but otherwise not noticing any related issues anymore.

@pepyakin
Copy link
Contributor

pepyakin commented Jan 7, 2025

Oh, sorry, I am retarded. The last few times I tested I was setting a breakpoint on serialize_workspace. Then it looks great!

I am wondering though what would the bulletproof fix look like. Just curiousity, feel free to ignore.

The comment re debounce makes sense, although I am not sure if I am 100% on board with it. It's not like Zed crashes that often, nor can I shutdown Zed in 300ms, nor I care saving the last 300ms of progress. cc @mikayla-maki

@SomeoneToIgnore
Copy link
Contributor

I do not have a very firm idea, but to handwave a bit around the good fix:

  • the serialization should compare previous serialized state with the new one and only write into DB if anything's changed

  • the serialization should react on event rather than a .serialize_workspace method call called somewhere outside of the workspace.rs

  • there has to be a basic test of user interactions that counts the serialization attempts (the hardest part, as our tests do not really construct the entire Zed as it's constructed during startup + we have no e2e tests like that yet)

@mikayla-maki
Copy link
Contributor

I'm not observing this specific issue with workspace serialization, and since @SomeoneToIgnore fixed some major cases of over writing, I'm going to close this issue for now. Let us know if it comes back! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug [core label] linux linux-x11 Linux X11 performance Feedback for performance issues, speed, memory usage, etc
Projects
None yet
Development

No branches or pull requests

8 participants