collect memory stats, but it's not really suitable #889
+205
−5
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
not suitable for.. a few reasons. but this does get reasonable numbers!
`propolis-server` memory use is in no small part... the binary itself.
given a 32 GiB VM with a few cores, this takes almost five seconds on
my workstation to get through all of /proc/{getpid()}/xmap. given
there being no I/O here, that's five seconds of CPU time to calculate
memory stats. not good, not something i'd want to merge.
additionally, while walking /proc/{getpid()}/xmap we at various points
hold the process' address space lock (for writing!), which can be very
much observed by the guest VM itself. i happened to log into the
above-mentioned VM while memory stat collection was going on, and login
hung for two seconds while that finished. steady-state behavior is less
bad (`while true; do date; sleep 0.2; done` does not show any
disruption for example), but, again, not shippable behavior without at
*least* a feature flag.
using `pmap -x <pid>` does similar operations with respect to the
system, opening `xmap` and reading entries and on, but it seems somewhat
faster than the code here. hundreds of milliseconds instead of plural
seconds. i'm not sure what's different, but the service-interrupting
nature of the metric collection makes it kind of a showstopper either
way.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
jclulow
reviewed
Mar 27, 2025
|
|
||
| // Safety: getpid() does not alter any state, or even read mutable state. | ||
| let mypid = unsafe { libc::getpid() }; | ||
| let xmap_path = format!("/proc/{}/xmap", mypid); |
There was a problem hiding this comment.
I think this could just be "/proc/self/xmap" ?
There was a problem hiding this comment.
i hoped and thought so, but i don't have a /proc/self* at all locally. i was kinda curious about it: if you have it maybe it's added by something downstream of illumos, and helios doesn't get the fun?
There was a problem hiding this comment.
pmooney informs me that /proc/self/ in fact there and just not in ls /proc (or, consequently, tab completion for /proc/s :) )
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
in case i'm doing anything funky with the approach here, but i think this is as good as it could get at the moment.
not suitable for.. a few reasons. but this does get reasonable numbers!
propolis-servermemory use is in no small part... the binary itself.given a 32 GiB VM with a few cores, this takes almost five seconds on my workstation to get through all of /proc/{getpid()}/xmap. given there being no I/O here, that's five seconds of CPU time to calculate memory stats. not good, not something i'd want to merge.
additionally, while walking /proc/{getpid()}/xmap we at various points hold the process' address space lock (for writing!), which can be very much observed by the guest VM itself. i happened to log into the above-mentioned VM while memory stat collection was going on, and login hung for two seconds while that finished. steady-state behavior is less bad (
while true; do date; sleep 0.2; donedoes not show any disruption for example), but, again, not shippable behavior without at least a feature flag.using
pmap -x <pid>does similar operations with respect to the system, openingxmapand reading entries and on, but it seems somewhat faster than the code here. hundreds of milliseconds instead of plural seconds. i'm not sure what's different, but the service-interrupting nature of the metric collection makes it kind of a showstopper either way.