The process details dialog displays more information on a process such as number of page faults and context switches. It also makes use of the (seemingly) unused lsofui library to display a list of files opened by the process. Currently it's only implemented for Linux.
Details
Details
- Reviewers
broulik davidedmundson mart
Not quite sure how to test this but we can always right click on any process in the process list and choose process details. lsofui does not work for other users' processes.
Diff Detail
Diff Detail
- Repository
- R111 KSysguard Library
- Lint
Lint Skipped - Unit
Unit Tests Skipped
Comment Actions
As for this patch.
I'm not particulalry convinced context switches is a very useful thing, ksysguard isn't catering to be a perf replacement.
But if it's hidden away in a dialog, meh, no real objections.
I don't think we need to have two independent dialogs. The existing "Show detailed memory information" and "Process details". I think they can be merged.
Comment Actions
The existing "Show detailed memory information" and "Process details". I think they can be merged.
The show detailed memory information is a QtWebKit-powered "plugin" (I would love to get rid of tbh)
Comment Actions
Yeah I would personally agree with that. Actually the "Show detailed memory information" dialog crashes ksysguard on my machine immediately (the back trace ends up with libQt5WebKit.so.5). I would try adding memory info to the new dialog and update the diff then.
Comment Actions
That crash should be fixed by [1] which I just saw forgot to merge to master
[1] https://cgit.kde.org/breeze.git/commit/?h=Plasma/5.13&id=6d886e9f75d04eb34cd34cac668606d027384f96
Comment Actions
Added detailed memory information to the new process details dialog. Like the original implementation, it displays a message if some of the items are not available.
(Also TIL ksysguard has scripting support... which didn't show up in the handbook whatsoever. It is pretty powerful indeed, but it seems an overkill to implement detailed memory info with scripting.)
Comment Actions
Any updates on whether this patch could be merged? It has been a long wait. If there are any improvements I can make to the patch I will be more than happy to put more work into it.
Comment Actions
I quite like this feature @chrisx
I hope you can still push it forward after all this time.
| processui/DetailsDlg.cpp | ||
|---|---|---|
| 1 ↗ | (On Diff #35891) | Personally I would favor a class name with no abbreviation, here DetailsDialog, and here perhaps make it more explicit like ProcessDetailsDialog. |
| processui/ksysguardprocesslist.cpp | ||
| 205 | Indentation seems off. | |
| 1225 | if (processes.size() != 1) { return; } | |
Comment Actions
I'll definitely update the patch if the maintainers show more interest in merging. After all I don't really like the idea of having WebKit (back when the patch was authored) or Chromium in my system monitor in order to view memory details.
... and I have to admit that my code style is a complete mess. I'll fix it in the next patch (if there is one).
Comment Actions
Rebased to master. Removed uses of deprecated class (QRegExp). Tidied the code up a bit.
Comment Actions
You have a few code indentation issues, that you can take care of.
https://phabricator.kde.org/D12827#inline-130952
And I think you should rename DetailsDlg to DetailsDialog, we prefer to avoid abbreviations in classes names as a convention.
Also the patch does not apply on master but I guess it depends on D23287 which also needs to be rebased on master...
Comment Actions
@meven Sorry I've totally forgotten about your suggestions when I was updating the patch...
I have renamed DetailsDlg to ProcessDetailsDialog. But I've got some minor issues with code indentation. I have tried out uncrustify-kf5 (which didn't even work out-of-box. Instead of formatting the files in-place, it produced *.uncrustify files instead. I have uncrustify 0.69, and I worked around the issue by adding --replace to the arguments for uncrystify in uncrustify-kf5). But the script produces way too many changes in some files I changed. One of those files is processes_linux_p.cpp, which uses both spaces and tabs for indentation. So at this moment I've only committed some of the indentation changes.
Any further help with pushing this patch forward would be greatly appreciated.
Comment Actions
Sorry I should have mentioned and better explained.
The uncrustify-kf5 is located kde-dev-scripts and depends on uncrustify.
To use it only for a few files a workaround is to run it git add only the files you mean to commit and then git checkout the files you don't want to chaneg.
If too many lines are concerned better skip this here then.
Please note I am no ksysguard maintainer, and the Ksysguard app is going through a UI renew.