Skip to content
This repository was archived by the owner on Sep 8, 2020. It is now read-only.

Add PSS (proportional set size), Swap and SwapPSS calculation on Linux#843

Open
ntninja wants to merge 2 commits intohishamhm:masterfrom
ntninja:master
Open

Add PSS (proportional set size), Swap and SwapPSS calculation on Linux#843
ntninja wants to merge 2 commits intohishamhm:masterfrom
ntninja:master

Conversation

@ntninja
Copy link
Copy Markdown

@ntninja ntninja commented Oct 9, 2018

I did not write this code and took a patch for this from https://github.com/linvinus/htop-mod. Please tell me about any improvements required.

While I can attest PSS to be a big improvement over RSS, there are some pain points about this:

  • /proc/<pid>/smaps is only readable by the (effective?) user of the given process. Since PSS calculation requires all memory mapping tables to be available for accurate calculation, this makes the PSS value somewhat useless htop runs as root. Actually this is not true making PSS data somewhat usable, but incomplete for users other than the current user, unless you're root. 🙁
  • Greatly improved on Linux 4.14+ thanks to @himikof's suggestion! (Crunching the smaps data noticeably slows down htop – far from unusable, but slower.)

Original code was written by Craig M. Brandenburg for htop 1.0.2
Many performance improvements by GitHub user linvinus, ported to htop 2.0.2

@himikof
Copy link
Copy Markdown

himikof commented Oct 16, 2018

Maybe the code could be extended to try smaps_rollup if available (in kernel since 4.14)? The format is exactly the same, just less data to parse.

@ntninja
Copy link
Copy Markdown
Author

ntninja commented Oct 16, 2018

@himikof: Thanks for the suggestion! I've implemented your suggestion and it does give quite a performance boost. Still not as quite snappy as when not reading any smaps file, but it stops the UI from feeling somewhat sluggish while PSS reporting is enabled.

@hishamhm
Copy link
Copy Markdown
Owner

Thanks for the patch! I still need to take some time to give it a spin, but on first glance my initial comment is that we'll probably not want to enable PSS by default, given the performance concerns.

@ntninja
Copy link
Copy Markdown
Author

ntninja commented Oct 30, 2018

I agree one that sentiment unfortunately. The original patch had it in by default, but given the speed on pre-4.14 kernels and the root requirement (that RSS and friends doesn't have) for complete reporting it is really a sane default currently. Let me know of any other things you notice and I'll apply the requested updates once there is more to do.

Comment thread linux/LinuxProcessList.c
Comment thread linux/LinuxProcessList.c

static bool LinuxProcessList_readSmapsFile(LinuxProcess* process, const char* dirname, const char* name, bool haveSmapsRollup) {
//http://elixir.free-electrons.com/linux/v4.10/source/fs/proc/task_mmu.c#L719
//kernel will return data in chunks of size PAGE_SIZE or less.
Copy link
Copy Markdown
Author

@ntninja ntninja Nov 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hishamhm: True and well analyzed! However, such reads will never happen for regular smaps because data is returned in chunks of PAGE_SIZE. For smaps_rollup, as you have correctly pointed out, this discussion is irrelevant anyways.

Regarding the verification of this claim: The linked code does appear to write out each result entry separately and when I run strace cat /proc/$$/smaps each read returns as many entries as fit into a 4k-page, without any “half-entries”.

(For the most part we have to trust the previous author's judgment here. Fortunately however we do not need to worry about kernel changes, since we're only really talking about Linux ~2.6.26 up to 2.13 here).

@ntninja
Copy link
Copy Markdown
Author

ntninja commented Dec 13, 2018

@hishamhm: I have already explained why this is not a problem in the comment above. Mind considering a merge again?

ntninja added 2 commits March 20, 2019 17:00
Original code was written by *Craig M. Brandenburg* for htop 1.0.2
Many performance improvements by GitHub user *linvinus*, ported to htop 2.0.2
@ntninja
Copy link
Copy Markdown
Author

ntninja commented Mar 20, 2019

@hishamhm: Ping

@natoscott
Copy link
Copy Markdown
Collaborator

Merged here: htop-dev/htop@f9625ca

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants