Bug 393911 - kwinrulesrc rewritten unnecessarily
Summary: kwinrulesrc rewritten unnecessarily
Status: RESOLVED FIXED
Alias: None
Product: kwin
Classification: Plasma
Component: rules (show other bugs)
Version: 5.12.4
Platform: Debian unstable Linux
: NOR grave
Target Milestone: ---
Assignee: KWin default assignee
URL: https://phabricator.kde.org/D12749
Keywords:
Depends on:
Blocks:
 
Reported: 2018-05-06 15:00 UTC by Antonio Russo
Modified: 2018-05-20 13:40 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In: 5.12.6
Sentry Crash Report:
mgraesslin: ReviewRequest+


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Antonio Russo 2018-05-06 15:00:24 UTC
~/.config/kwinrulesrc appears to be modified (stat shows changed times) every time a window is created or destroyed that matches a rule in that file.

This is (obviously) not great for SSD lifetimes, power consumption, etc., but---making things worse---kwin happens to *block* on this write, so the whole desktop freezes. This is particularly painful on devices that are a slow.

[This seems to be distinct from #347111.]
Comment 1 Martin Flöser 2018-05-06 17:04:59 UTC
The reason for the behavior is that there are temporary rules which might be removed. In those cases we need to sync.

But if the rules didn't change there is no need to sync. That can hopefully be improved. Not sure about not blocking - iirc kconfig is not thread save.
Comment 2 Martin Flöser 2018-05-06 19:30:37 UTC
> ~/.config/kwinrulesrc appears to be modified (stat shows changed times) every time a window is created

I could confirm this by reading the code. This happens from discard used rules to remove force temporary rules. But it doesn't check whether there are any temporary rules at all and triggers an update for saving.
Comment 3 Martin Flöser 2018-05-06 20:07:59 UTC
Also window removing triggers discardUsed. So it's just one place to fix it all and I have already a patch.
Comment 4 Martin Flöser 2018-05-07 20:05:40 UTC
Patch at: https://phabricator.kde.org/D12749

If you could test and verify that the issue is fixed, this would be appreciated.
Comment 5 Antonio Russo 2018-05-07 22:18:23 UTC
Thanks! After patching, I'm not seeing the unnecessary updates anymore. (The original symptom was *terrible* lag when opening new Konsole windows during a ZFS scrub)
Comment 6 Martin Flöser 2018-05-08 04:14:08 UTC
Great! Thanks for testing
Comment 7 Martin Flöser 2018-05-20 13:40:52 UTC
Git commit 9a02ed4d360ffa18c3c406ab0eb1d01ccc9c0901 by Martin Flöser.
Committed on 20/05/2018 at 13:40.
Pushed by graesslin into branch 'Plasma/5.12'.

Do not save kwinrulesrc on every window opening/closing

Summary:
Our rule handling has had a grave error for years. Whenever a window
with a rule was openend or closed the kwinrulesrc was written back to
disk.

The reason for this behavior is that temporary rules need to be discarded
once they were used. For that there is a method discardUsed which invokes
requestDiskStorage whenever a rule for the window was found. But it did
not check whether there was a rule requiring this.

This change modifies the discardUsed to track whether it changed a rule
and only writes back in case there was a change.
FIXED-IN: 5.12.6

Test Plan: Only compile tested

Reviewers: #kwin, #plasma

Subscribers: kwin

Tags: #kwin

Differential Revision: https://phabricator.kde.org/D12749

M  +13   -5    rules.cpp
M  +1    -1    rules.h

https://commits.kde.org/kwin/9a02ed4d360ffa18c3c406ab0eb1d01ccc9c0901