-
-
Notifications
You must be signed in to change notification settings - Fork 326
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Process memory Enhancement [Linux] #1377
Comments
Seems like |
Yes smaps_rollup does require the process to run in privileged mode. Some of us use the sysinfo library in privileged mode anyway, wouldn't this benefit the folks who are okay with running in privileged mode? |
It requires more code in order to work so I'm not very much in favour of it. |
True that any new feature requires more code to work, but what is it about the code we will add for this feature that makes you go not in favor of it? |
Btw i really appreciate the work you have done here. Big fan of this library. |
We need to read the new file and if it fails, fall back to the current one. Which means that for your processes, you'll get different kind of information than others'. That also means that we'll try to read even more file compared to the current situation. Another approach would be to detect if the current user has access to all of these files, but I don't really see how since I'm not sure root users can have access to everything, including other root users processes. |
I am proposing something slightly different than what you are envisioning. Assume we introduce With this approach, you can provide consistent behavior across platforms and allow each platform to provide their own "memory extension counters" if need arises. |
But then it's something only available on linux targets, which is pretty bad for a unified API. |
Is that so bad? Others will just get an empty HashMap and as long as it's well documented, do we see an issue? I would argue that if some platforms provide better more accurate metrics, we should provide that information even if its optional only to that platform. |
Well, that's two reasons not in favour of adding this feature. Adding more API if only one OS can use it is really something I want to avoid as much as possible. |
I noticed sysinfo uses
/proc/<pid>/statm
to get the memory details like virtual memory and rss. As the man page suggests the rss reported in statm is not accurate and suggests usingsmaps_rollup
instead. smaps_rollup file is pretty small one and provides a lot more information like PSS.In reality, pss is a lot more useful memory metric than rss when there's a lot of shared memory involved in the system. So by changing where we get the memory stats from we can actually kill two birds in one stone. One caveat however is that, windows has no concept of PSS and I am not sure how we can expose these additional metrics available only in "unix" systems.
Maybe we can support
process.memory_ext()
that provides all the fields from smaps_rollup on demand.Thoughts?
The text was updated successfully, but these errors were encountered: