From 100% plasma cpu usage to normal when using vivaldi
AbandonedPublic

Authored by jtamate on Feb 6 2018, 2:48 PM.

Details

Reviewers
hein
Group Reviewers
Plasma: Workspaces
Summary

Cache the calls to windowUrlFromMetadata, because it can be called quite often, for example with the vivaldi browser started, and it usually calls KServiceTypeTrader::defaultOffers that also involves I/O.

CCBUG: 342056
CCBUG: 358231

Test Plan

Browse with vivaldi browser in private navigation.

Diff Detail

Repository
R120 Plasma Workspace
Lint
Lint Skipped
Unit
Unit Tests Skipped
jtamate created this revision.Feb 6 2018, 2:48 PM
Restricted Application added a project: Plasma. · View Herald TranscriptFeb 6 2018, 2:48 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
jtamate requested review of this revision.Feb 6 2018, 2:48 PM
jtamate edited the summary of this revision. (Show Details)Feb 6 2018, 2:58 PM
broulik added a subscriber: broulik.Feb 6 2018, 2:59 PM

Interesting. I'm curious as to why it calls those functions that often in the first place while copying files.

Interesting. I'm curious as to why it calls those functions that often in the first place while copying files.

My guess is that it is used to fill the task completion in the task bar, but is only a wild guess.

The progress display is completely independent of libtaskmanager. Can you disable it (Uncheck "Show application badges and progress" in task manager settings) and see if that changes anything?

jtamate updated this revision to Diff 26692.Feb 7 2018, 11:01 AM
jtamate edited the test plan for this revision. (Show Details)

What I've noticed is that with the first version of the patch,
every time I changed konsole tabs, saved a file in kate, the appDataFromUrl (missed the cache) was called.
After not removing that data from the cache, with an appdata of 0.04%, the calls to appDataFromUrl are far, far away now (in the 0.01%) along with the KService... calls.
Does it have unexpected consequences? I don't know the answer, but I've not noticed them.

I'll check disabling "Show application badges and progress" (without this second version).

every time I changed konsole tabs, saved a file in kate, the appDataFromUrl (missed the cache) was called.

Whenever NET::WMVisibleName changes, both caches are evicted. It might be worth checking what's more expensive to create, the KWindowInfo or AppData and then use one or the other depending on it, e.g. use the KWindowInfo for the visibleName and don't nuke the appDataCache when that changes, or so. I'm not very familiar with that codebase.

libtaskmanager/xwindowtasksmodel.cpp
330

However, just *not* killing the cache would lead to DisplayRole, AppId, AppName, GenericName not updating as these are taken from appData in data()

jtamate updated this revision to Diff 26937.Feb 11 2018, 3:11 PM
jtamate edited the test plan for this revision. (Show Details)

In https://phabricator.kde.org/D9840 I noticed a high i/o and cpu usage
when using the vivaldi browser, opening 10 tabs just caused a 40% cpu
usage. Now, opening 10 tabs or more is bellow the 0.03%.

In https://phabricator.kde.org/D9840 I noticed a high i/o and cpu usage
when using the vivaldi browser, opening 10 tabs just caused a 40% cpu
usage. Now, opening 10 tabs or more is bellow the 0.03%.

I know now why it is calling windowUrlFromMetadata.
Because unlike firefox and chromium, with sub processes using the same executable but with parameters, the vivaldi sub processes are called vivaldi-bin, and there is no .desktop file for that executable name. As soon as I've manually created a .desktop file for vivaldi-bin, plasmashell cpu became normal without the patch. With the patch, there is no need for the .desktop file.

mwolff added a subscriber: mwolff.Feb 14 2018, 9:36 AM
mwolff added inline comments.
libtaskmanager/xwindowtasksmodel.cpp
523

const auto url = ...;

note the space after the =

jtamate updated this revision to Diff 27177.Feb 14 2018, 6:42 PM
  • From 1.03% to 0.08% cpu usage moving 50.000 files

fixed mwolf comment

jtamate marked 2 inline comments as done.Feb 14 2018, 6:42 PM
cfeck added a reviewer: hein.Feb 14 2018, 6:53 PM
jtamate edited the summary of this revision. (Show Details)Feb 15 2018, 9:09 AM
jtamate edited the test plan for this revision. (Show Details)
mart added a subscriber: mart.Mar 23 2018, 2:49 PM

any update on this?

jtamate updated this revision to Diff 36978.Jul 1 2018, 6:50 AM
jtamate retitled this revision from From 1.03% to 0.08% cpu usage moving 50.000 files to From 100% plasma cpu usage to normal when using vivaldi.
jtamate edited the summary of this revision. (Show Details)
jtamate edited the test plan for this revision. (Show Details)

Implement only the windowUrlFromMetadata cache.
It avoids a 100% plasma cpu usage when using vivaldi in private navigation.

hein requested changes to this revision.Jul 2 2018, 10:21 AM

This patch doesn't make any sense. It's setting up a cache for something computed from data that's subject to change, and it's never evicting it when that data changes.

This revision now requires changes to proceed.Jul 2 2018, 10:21 AM
In D10342#286023, @hein wrote:

This patch doesn't make any sense. It's setting up a cache for something computed from data that's subject to change, and it's never evicting it when that data changes.

To make it right, should it also be wiped in XWindowTasksModel::Private::windowChanged?
Or perhaps is it better to try to create that cache in KServiceTypeTrader?

Or perhaps is it better to try to create that cache in KServiceTypeTrader?

That will be the right place, reading from disk is always costly, it will be better to make service parsing on file changes otherwise to read from cache.

jtamate abandoned this revision.Jul 3 2018, 9:40 AM