Differential D23205

[KProcessList] Optimize KProcessList::processInfo
ClosedPublic
Actions

Authored by hallas on Aug 16 2019, 4:28 PM.

Details

Reviewers
davidedmundson
broulik
mpyne
Commits
R244:e16dda4d50bf: [KProcessList] Optimize KProcessList::processInfo
Summary

Optimize KProcessList::processInfo on unix so that it doesn't iterate over all
processes and then filter the list to the requested process. Instead refactor
the code that fetches process info from a single process and use that function.

Test Plan

Unit Test

BUG: 410945

Diff Detail

Repository
R244 KCoreAddons
Branch
optimize_kprocesslist_processinfo
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 15187
Build 15205: arc lint + arc unit
hallas created this revision.Aug 16 2019, 4:28 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptAug 16 2019, 4:28 PM
hallas requested review of this revision.Aug 16 2019, 4:28 PM
apol added a subscriber: apol.Aug 18 2019, 12:27 AM

patch looks good overall.

src/lib/util/kprocesslist_unix.cpp
157

checking if it's empty is unnecessary.

171

Use QString::number()

hallas updated this revision to Diff 64913.Aug 29 2019, 5:52 AM
hallas marked an inline comment as done.

Fixed review comments, rebased.

hallas marked an inline comment as done.Aug 29 2019, 5:52 AM
hallas updated this revision to Diff 64914.Aug 29 2019, 5:53 AM

Add bug reference

hallas edited the test plan for this revision. (Show Details)Aug 29 2019, 5:55 AM
mpyne accepted this revision.Nov 24 2019, 6:16 PM
mpyne added a subscriber: mpyne.

I've reviewed the patch and it's good, you've addressed the issues that were noted by Aleix already, so please commit and we can address anything that might pop up as it happens.

This revision is now accepted and ready to land.Nov 24 2019, 6:16 PM
hallas closed this revision.Dec 15 2019, 6:53 AM
kossebau added a subscriber: kossebau.Dec 15 2019, 6:51 PM

I pushed a quick fix to unbreak the build as bfdc20ed7c6fc1397ba66edd07f86c84d380e291
You might want to inspect it to add that optimization code for the procstat as well, if possible (no idea).

hallas added a comment.Dec 15 2019, 7:09 PM

Sorry for breaking the build :/

@kossebau - thanks for fixing it so quickly! I think your fix looks fine :D

Revision Contents
Changeset List

Diff 63873

View Options

src/lib/util/kprocesslist.cpp

Loading...
View Options

src/lib/util/kprocesslist_unix.cpp

Loading...
View Options

src/lib/util/kprocesslist_win.cpp

Loading...