Add a new daemon for stats monitoring
ClosedPublic

Authored by davidedmundson on Mar 27 2020, 10:53 AM.

Details

Summary

ksysguardd, whilst good, has a few problems

The code is a bit archaic, it relies on a polling API, which is overhead
for infrequently changed values or where setting up a monitor has a big
overhead.

It also moves the problem of translations into the daemon, allowing for
better extensibility without requiring client side changes.

The daemon is based around a typical OO model. Plugins have lists of
objects, those objects have properties using common Qt patterns. A
property also has various metadata.

For full compatibility ksgrd is wrapped and the plan is to land with the
bridge, then slowly land patches that use the new API natively.

An nvidia plugin is also added to show the API being used in another
format.

This is all consumed by the new API posted in D28141

Test Plan

Unit test
Used with the new library to create a new suite of applet (upcoming patch)
Used in a ported ksysguard

Diff Detail

Repository
R106 KSysguard
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
Restricted Application added a project: Plasma. · View Herald TranscriptMar 27 2020, 10:53 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
davidedmundson requested review of this revision.Mar 27 2020, 10:53 AM
plugins/global/nvidia/nvidiaplugin.json
1 ↗(On Diff #78625)

To address this, I don't use the json at all, as it's itself a factory class which provides metadata.

But I couldnt' get rid of it without error.

zzag added a subscriber: zzag.Mar 27 2020, 11:08 AM
zzag added inline comments.
plugins/global/nvidia/nvidia.cpp
3–4

Typo?

Drop object library, it was too cool for some cmake versions

Register bus name after loading plugins

mart added a subscriber: mart.Apr 21 2020, 9:07 AM
mart added inline comments.
kstats/ksysguard_iface.xml
24

since this is array in/array out, could consistence of naming be improved?
either
sensorsIds/sensorsInfos
or
sensorIds/sensorInfos

kstats/ksysguarddaemon.cpp
59

needs to be always polling every half a second? could it be somewhat adjustable (maybe depending on ac/on battery?)

87

if (

kstats/test/main.cpp
2

copyright?

58

what's the commandline option missing?

libkstats/AggregateSensor.cpp
200

if this timer is needed for compression, shouldn't be the same timer always restarted?
if i call singleshot 10 times, iirc that slot will be invoked delayed 10 times

plugins/global/ksgrd/ksgrdiface.cpp
186

if () {
}

379

maybe mapping on a config file?

ivan added a subscriber: ivan.Apr 21 2020, 9:16 AM
ivan added inline comments.
kstats/autotests/main.cpp
46

setMax?

kstats/client.cpp
39
57

Maybe:

if(auto sensor = m_daemon->findSensor(sensorPath)) {
    ...
}
ivan added inline comments.Apr 21 2020, 9:25 AM
kstats/client.cpp
66–67

... = sensor->info();

80–81

if (auto sensor = ...)

kstats/ksysguarddaemon.cpp
75

[=] is evil

112–114

clang-format

162

I'm not going to be bold enough to propose

if (const auto variant = ...; variant.isValid()) { }
173–174

const int, const int

ahiemstra added inline comments.
kstats/client.cpp
46

This should probably use qAsConst.

kstats/ksysguarddaemon.cpp
77

Should we do some error logging here? Might help future plugin development.

94
const auto containers = provider->containers();
for(auto container : containers) {
112

qAsConst(m_containers)

113

Like above, objects() should be in a const variable. (Also for sensors on the line below).

202

qAsConst(m_clients)

libkstats/AggregateSensor.cpp
200

It's guarded with "m_dataChangeQueued" which only gets reset after the timer triggers.

libkstats/SensorContainer.cpp
65

Uh, so shouldn't we fix this?

libkstats/SensorContainer.h
57

Maybe move this to private with friend class SensorObject? Though I don't really see why this is internal only. Also probably should rename it to addObject?

davidedmundson marked 23 inline comments as done.

all the changes

davidedmundson marked an inline comment as done.Apr 24 2020, 11:09 AM
davidedmundson added inline comments.
kstats/ksysguarddaemon.cpp
59

Indeed, the current code is every 2s. I ramped it up to expose any issues.

I would quite like it to be somewhat container specific.
i.e disk storage doesn't change much, CPU is more volatile.

I'll change it, and then maybe we can revisit this topic once some more things are ported.

libkstats/SensorContainer.cpp
65

Seems like a plan :)

libkstats/SensorContainer.h
57

It was just to explicitly enforce the memory management combined with the tree management.

I'll go with private, then we can change if we find we need to

plugins/global/ksgrd/ksgrdiface.cpp
379

Plan (with some pending code) is to just move these in natively into a more readable plugin

davidedmundson marked an inline comment as done.

Clear up plugin loading

mart accepted this revision.May 4 2020, 12:14 PM
This revision is now accepted and ready to land.May 4 2020, 12:14 PM
ngraham accepted this revision.May 4 2020, 2:19 PM
ahiemstra accepted this revision.May 6 2020, 2:21 PM

Let's do this.

This revision was automatically updated to reflect the committed changes.