Register spawned applications as an independent cgroups

Authored by davidedmundson on Mar 6 2020, 9:02 AM.



Having each application in it's own scope brings numerous advantages:

  • PIDs are very yesteryear, a modern application has a tonne of

processes. We want some sort of logical grouping in ksysguard.
We have a working version of this.

It also can resolve the case of correctly matching the
audio indicator to the correct application in the task manager.
Something that has been proven to not work reliably by tracking parent

  • We can use the cgroup freezer controller on plasma-mobile to suspend

background processes.

  • We want to put things into the correct slice.

Future systemd will split user.slice into 3 subslices, background
services, apps and chrome (with chrome being plasmashell and kwin in our
case). Putting applications in the correct slice will bring pre-bumped
niceness levels.

  • We also can expose cgroup resources limits
  • Things get cleaned up logout without relying on xsmp.

This is analogous to Gnome's recent change:

It was also discussed in person at a very-mini meeting at FOSDEM with
both systemd and gnome people.

Task T12678

If a relevant cgroup controller (systemd) is not running, this simply
no-ops without issue.

In addition things are guarded by an environment variable so we don't
introduce any behavioural changes to released Plasma's.

This change is incredibly minimal by launching as normal and then
tagging afterwards. It's a bit of a chicken and egg scenario as we merge
the intended usages of this change.

After things are established we will want to move spawning the
application to be responsiblity of the cgroup controller as transient
services rather than transient scopes. It will be more "technically
correct" and allow even more features such as improved logging.

Test Plan

Ran app from kickoff, started systemd-cgls
I've been using some variant of this on a private work project for months
Also ran against pending ksysguard modifications that showed applications grouped nicely

Diff Detail

R241 KIO
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.
davidedmundson created this revision.Mar 6 2020, 9:02 AM
Restricted Application added a project: Frameworks. · View Herald TranscriptMar 6 2020, 9:02 AM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
davidedmundson requested review of this revision.Mar 6 2020, 9:02 AM
apol added a subscriber: apol.Mar 9 2020, 1:56 AM
apol added inline comments.
51 ↗(On Diff #77071)

This could be in the function scope as static.

281 ↗(On Diff #77071)

Who defines this environment variable?

301 ↗(On Diff #77071)

Declare and define together

302 ↗(On Diff #77071)


Also I'm not sure I understand...

308 ↗(On Diff #77071)

Does this need to be sync?

281 ↗(On Diff #77071)

I figured startplasma?

It's just so we don't change existing code with new frameworks

302 ↗(On Diff #77071)

Maybe unused wasn't the right term.

I was saying this list will be empty. Maybe that's an unhelpful comment as you can see it's empty from the code.

davidedmundson marked 6 inline comments as done.Mar 19 2020, 2:13 PM

rebase on David F's work

Relevant docs on the spec we're moving towards:

dfaure requested changes to this revision.Mar 21 2020, 10:59 AM
dfaure added inline comments.

The code in this method should be in some #ifdef UNIX and not MAC, right?
Pretty pointless to make this DBus call on Windows ;)


(this is what made me think about Windows, where pid is a 64bit value).


And then? if we don't care about the async reply, maybe you should remove the left part of the equal sign?

This revision now requires changes to proceed.Mar 21 2020, 10:59 AM

review comments

davidedmundson marked 3 inline comments as done.Mar 23 2020, 1:13 PM
dfaure accepted this revision.Mar 24 2020, 9:04 PM

It would be good to add a link to the spec as a code comment -- but I see that the docu isn't merged yet, so can't be done yet.


unused? std::call_once and std::once_flag come from <mutex>



This revision is now accepted and ready to land.Mar 24 2020, 9:04 PM
This revision was automatically updated to reflect the committed changes.