Fix high I/O writes with shadow state#324
Conversation
|
|
ec186dc to
bf519eb
Compare
|
I don't understand how the In addition, I find it hard to believe that debouncing saves for I believe that the bulk of the improvement introduced by this PR is likely from the processes of reading the files and comparing the contents to skip useless writes, as well as indefinitely delaying setImage-induced saves. Could you remove the |
bf519eb to
24c15d9
Compare
|
Hi, thanks for the review :) I partially agree. You were right about For In my test, keeping only the file comparisons reduced writes, but introduced many reads and still left repeated profile/image writes. With BeforeWith file compare onlyWith
|
But as far as I know, setTitle, setImage and setState updates are intended to be persisted across restarts of OpenDeck. If I'm wrong about that, feel free to correct me with some evidence, but otherwise, discarding those changes entirely doesn't seem correct to me. |
The state, title, and images are indeed saved persistently, though only after two seconds without further changes or when the program terminates. In my view, there is really no other solution, as the setter functions would otherwise trigger an immediate change on the hard drive. |
|
Yes, but with this code, iirc, they are never saved (not after 2 seconds and not when it terminates - persisted_states is discarded). That's just from my memory from looking at this last though |
|
I tested it and the let mut states = value.persisted_states.clone();In line 135 the variable gets part of the returned struct which is serialized to JSON later: Self {
context: disk_context,
action: value.action,
states,
current_state: value.current_state,
settings: value.settings,
children: value.children.map(|c| c.into_iter().map(|v| v.into()).collect()),
}That being said, I totally understand your caution. I'm not deeply familiar with the codebase, and my implementation could have flaws, that I can't see. Would it be possible for you or another maintainer to take over this issue? You could use this PR as inspiration and create a more well-thought-out fix :) |
|
Ah, I see it there! OK, that makes more sense then. There are no other maintainers ;) but I'll keep this open and review it when I get time. Currently taking a break from this project to work on something new. |
Oh, then an extra thank you for your service! 🫡
I understand, variety is important. When there is time, I will also examine the effects of the changes in more detail. I could formulate a few tests and run review agents over it to get a deeper understanding or something like that. |
3 participants
This PR reduces excessive disk writes caused by frequent plugin-driven button updates (Issue: #249).
Previously, every dynamic
setTitle,setImage, orsetStateupdate mutated the persisted profile state and often caused a full profile JSON write plus repeated exported image writes. This was especially expensive for monitor-style plugins that update buttons multiple times per second.Now, transient plugin display updates stay in runtime state, while persisted profile state is kept separately. Real configuration changes still persist, but writes are skipped or debounced where possible.
Changes
ActionInstanceinto runtime display state and persisted state.setTitleandsetImagefrom plugins no longer persist transient display updates.setStateremains persistent, but now uses debounced profile saving.setSettingswrites are debounced and skipped when unchanged.data:images are only rewritten when file contents changed.Write Behavior
Even when a debounced profile save fires,
*.json.tempis only written if the serialized profile differs from the existing file. Exporteddata:images are only rewritten if the file content changed.Before
flowchart TD A[Plugin updates button] --> B[setTitle / setImage / setState] B --> C[Mutate profile state] C --> D[Render updated button] C --> E[Save full profile JSON] E --> F[Write *.json.temp] E --> G[Export data: images to config/images] G --> H[Rewrite SVG/PNG files repeatedly]After
flowchart TD A[Plugin setTitle / setImage] --> B[Mutate runtime state only] B --> C[Render updated button] B --> D[0 profile writes] E[Plugin setState] --> F[Update runtime current_state] F --> G[Update persisted current_state] G --> H[Debounced profile save after 2s idle] I[Plugin/PI setSettings] --> J{Settings changed?} J -- No --> K[No save scheduled] J -- Yes --> H L[Editor/UI change] --> M[Immediate profile save] H --> N{Serialized profile changed?} M --> N N -- No --> O[Skip disk write] N -- Yes --> P[Write *.json.temp] P --> Q{Exported image changed?} Q -- No --> R[Skip image rewrite] Q -- Yes --> S[Write image file]How I did it?