D17217: Implement free memory notifier

[Oleg Solovyov]

McPain created this revision.
McPain added a reviewer: broulik.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
McPain requested review of this revision.

REVISION SUMMARY
FreeMemoryNotifier will warn when free RAM is running out.
By default, it is 25% without swap space.

Like freespacenotifier, it will check every 5 seconds for amount of free memory and
warn every time when it drops below limit and when free space amount drops more than half of previous value

User can either kill most greedy process immediately or open a task manager to kill unneeded processes manually.
Task manager was patched to ignore global config and show only user own processes, sorted by memory used in descending order.

The settings are available at new KCM located in Desktop Behavior category.
KCM and KDED modules are linked through DBus - committed settings are applied immediately.

REPOSITORY
R120 Plasma Workspace

REVISION DETAIL
https://phabricator.kde.org/D17217

AFFECTED FILES
plasma-workspace/CMakeLists.txt
plasma-workspace/freememorynotifier/CMakeLists.txt
plasma-workspace/freememorynotifier/COPYING
plasma-workspace/freememorynotifier/Messages.sh
plasma-workspace/freememorynotifier/README
plasma-workspace/freememorynotifier/freememorynotifier.cpp
plasma-workspace/freememorynotifier/freememorynotifier.desktop
plasma-workspace/freememorynotifier/freememorynotifier.h
plasma-workspace/freememorynotifier/freememorynotifier.kcfg
plasma-workspace/freememorynotifier/freememorynotifier.notifyrc
plasma-workspace/freememorynotifier/freememorynotifier_prefs_base.ui
plasma-workspace/freememorynotifier/kcm_freememorynotifier.cpp
plasma-workspace/freememorynotifier/kcm_freememorynotifier.desktop
plasma-workspace/freememorynotifier/kcm_freememorynotifier.h
plasma-workspace/freememorynotifier/module.cpp
plasma-workspace/freememorynotifier/module.h
plasma-workspace/freememorynotifier/org.kde.FreeMemoryNotifier.xml
plasma-workspace/freememorynotifier/settings.kcfgc
plasma-workspace/systemmonitor/ksystemactivitydialog.cpp
plasma-workspace/systemmonitor/ksystemactivitydialog.h
plasma-workspace/systemmonitor/main.cpp

To: McPain, broulik
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

[Nathaniel Graham]

ngraham added a reviewer: VDG.
ngraham added a comment.


Thanks for all the work!

...But is this actually useful or actionable for the majority of users? Free space is a fairly understandable concept: if you run out, you need to delete things to make more room before you can add more stuff. It won't fix itself unless the user does something. But memory pressure requires a much greater technical understanding and isn't subject to the same conditions. When the system is using up 75% of the available memory, there isn't necessarily a problem at all. Apps will move to swap automatically. Even if there is a problem, it's only temporary, and it may fix itself if the user does nothing as the system shuffles things around.

I worry that this notifier would just be yet another annoying pop-up that people dismiss because they don't understand it.

REPOSITORY
R120 Plasma Workspace

REVISION DETAIL
https://phabricator.kde.org/D17217

To: McPain, broulik, #vdg
Cc: ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

[Oleg Solovyov]

McPain added a comment.

Post by Nathaniel Graham
Thanks for all the work!
...But is this actually useful or actionable for the majority of users? Free space is a fairly understandable concept: if you run out, you need to delete things to make more room before you can add more stuff. It won't fix itself unless the user does something. But memory pressure requires a much greater technical understanding and isn't subject to the same conditions. When the system is using up 75% of the available memory, there isn't necessarily a problem at all. Apps will move to swap automatically. Even if there is a problem, it's only temporary, and it may fix itself if the user does nothing as the system shuffles things around.
I worry that this notifier would just be yet another annoying pop-up that people dismiss because they don't understand it.

Sometimes I see what happens if you're running out of memory and oom can't do anything - the whole system halts and only solution for this is hard reset with all unsaved data lost.
25% is not hardcoded value, you can reduce threshold down to 1% and also you can include your swap space in settings.

REPOSITORY
R120 Plasma Workspace

REVISION DETAIL
https://phabricator.kde.org/D17217

To: McPain, broulik, #vdg
Cc: ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

[Andres Betts]

abetts added a comment.


Is having this feature an optional feature? Can it be turned off?

REPOSITORY
R120 Plasma Workspace

REVISION DETAIL
https://phabricator.kde.org/D17217

To: McPain, broulik, #vdg
Cc: abetts, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

[Oleg Solovyov]

McPain added a comment.

Post by Andres Betts
Is having this feature an optional feature? Can it be turned off?

I'll make the notification "disableable" tomorrow

REPOSITORY
R120 Plasma Workspace

REVISION DETAIL
https://phabricator.kde.org/D17217

To: McPain, broulik, #vdg
Cc: abetts, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

[Nathaniel Graham]

ngraham added a comment.

Post by Oleg Solovyov
Sometimes I see what happens if you're running out of memory and oom can't do anything - the whole system halts and only solution for this is hard reset with all unsaved data lost.
25% is not hardcoded value, **you can reduce threshold down to 1%** and also **you can include your swap space in settings.**

The problem is that none of this is appropriate to expose regular users to. If this feature goes in, it needs //perfect// defaults, because 99.99% of users will never change the defaults even if they are annoyed by them.

I would recommend changing the default threshold from 25% to 10%, and automatically detect swap space rather than making the user configure it.

REPOSITORY
R120 Plasma Workspace

REVISION DETAIL
https://phabricator.kde.org/D17217

To: McPain, broulik, #vdg
Cc: abetts, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

[Martin Flöser]

graesslin added a comment.


I don't think that this is useful - sorry. I run out of memory about once every five years, so polling every five seconds is way too often and a waste of resources. On the other hand the time when it happens polling 5 sec is way too low as then we are already dead. This can only happen if an application eats memory and then it's going fast.

REPOSITORY
R120 Plasma Workspace

REVISION DETAIL
https://phabricator.kde.org/D17217

To: McPain, broulik, #vdg
Cc: graesslin, abetts, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

[Nathaniel Graham]

ngraham added a comment.

Post by Martin Flöser
I don't think that this is useful - sorry. I run out of memory about once every five years, so polling every five seconds is way too often and a waste of resources. On the other hand the time when it happens polling 5 sec is way too low as then we are already dead. This can only happen if an application eats memory and then it's going fast.

Agreed.

REPOSITORY
R120 Plasma Workspace

REVISION DETAIL
https://phabricator.kde.org/D17217

To: McPain, broulik, #vdg
Cc: graesslin, abetts, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

[David Edmundson]

davidedmundson added a comment.


Given there is some negativity in the feedback I do want to point out that it's cool to still upload whatever to the KDE store/wider ecosystem. Not everything run in plasma needs to be from plasma; we go to great lengths to make sure that's the case.

There's also an alternative that I'd like to suggest:
According to https://www.kernel.org/doc/Documentation/cgroup-v1/memory.txt there's a system called "memory pressure" you can watch for events on a specific FD with various levels of out-of-memory-ness. A lot cheaper than polling, far more reliable, and following the system's OOM configuration. Getting that in at a frameworks level so it can be hooked up to notifications in this case, but also garbage collections in UI code could be pretty cool.

INLINE COMMENTS

freememorynotifier.cpp:55
+ connect(&timer, &QTimer::timeout, this, &FreeMemoryNotifier::checkFreeMemory);
+ timer.start(1000 * 5 /* 1 minute */);
+}

how is that 1 minute?

REPOSITORY
R120 Plasma Workspace

REVISION DETAIL
https://phabricator.kde.org/D17217

To: McPain, broulik, #vdg, ngraham
Cc: davidedmundson, graesslin, abetts, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

[Oleg Solovyov]

McPain added a comment.

Post by David Edmundson
According to https://www.kernel.org/doc/Documentation/cgroup-v1/memory.txt there's a system called "memory pressure" you can watch for events on a specific FD with various levels of out-of-memory-ness. A lot cheaper than polling, far more reliable, and following the system's OOM configuration. Getting that in at a frameworks level so it can be hooked up to notifications in this case, but also garbage collections in UI code could be pretty cool.

Thanks. Any examples in KDE? I can't implement this from scratch right now.

REPOSITORY
R120 Plasma Workspace

REVISION DETAIL
https://phabricator.kde.org/D17217

To: McPain, broulik, #vdg, ngraham
Cc: davidedmundson, graesslin, abetts, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

[Oleg Solovyov]

McPain updated this revision to Diff 46892.
McPain added a comment.


Default limit: 25% -> 10%
Fix typos
Auto detect swap, removed "includeSwap" setting
KSysGuard: show kill button (implemented in D17366 <https://phabricator.kde.org/D17366>)

CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D17217?vs=46407&id=46892

REVISION DETAIL
https://phabricator.kde.org/D17217

AFFECTED FILES
plasma-workspace/CMakeLists.txt
plasma-workspace/freememorynotifier/CMakeLists.txt
plasma-workspace/freememorynotifier/COPYING
plasma-workspace/freememorynotifier/Messages.sh
plasma-workspace/freememorynotifier/README
plasma-workspace/freememorynotifier/freememorynotifier.cpp
plasma-workspace/freememorynotifier/freememorynotifier.desktop
plasma-workspace/freememorynotifier/freememorynotifier.h
plasma-workspace/freememorynotifier/freememorynotifier.kcfg
plasma-workspace/freememorynotifier/freememorynotifier.notifyrc
plasma-workspace/freememorynotifier/freememorynotifier_prefs_base.ui
plasma-workspace/freememorynotifier/kcm_freememorynotifier.cpp
plasma-workspace/freememorynotifier/kcm_freememorynotifier.desktop
plasma-workspace/freememorynotifier/kcm_freememorynotifier.h
plasma-workspace/freememorynotifier/module.cpp
plasma-workspace/freememorynotifier/module.h
plasma-workspace/freememorynotifier/org.kde.FreeMemoryNotifier.xml
plasma-workspace/freememorynotifier/settings.kcfgc
plasma-workspace/systemmonitor/ksystemactivitydialog.cpp
plasma-workspace/systemmonitor/ksystemactivitydialog.h
plasma-workspace/systemmonitor/main.cpp

To: McPain, broulik, #vdg, ngraham
Cc: davidedmundson, graesslin, abetts, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart