Skip to content
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

total_rss_bytes and total_pss_bytes from smaps_rollup #1134

Merged

Conversation

blt
Copy link
Collaborator

@blt blt commented Dec 6, 2024

What does this PR do?

This commit adds collection of total PSS bytes for all processes from
smaps_rollup, adjusting total_rss_bytes to also be from the same source

@blt blt mentioned this pull request Dec 6, 2024
@blt blt marked this pull request as ready for review December 6, 2024 02:43
@blt blt requested a review from a team as a code owner December 6, 2024 02:43
@blt blt force-pushed the blt/slim_out_joinset branch from d0752e4 to 917bab0 Compare December 6, 2024 02:51
@blt blt force-pushed the blt/total_rss_bytes_and_total_pss_bytes_from_smaps_rollup branch from 6602a68 to 9b8dc34 Compare December 6, 2024 02:52
@blt blt force-pushed the blt/slim_out_joinset branch from 917bab0 to 38e96c1 Compare December 6, 2024 02:57
@blt blt force-pushed the blt/total_rss_bytes_and_total_pss_bytes_from_smaps_rollup branch from 9b8dc34 to 84cd41c Compare December 6, 2024 02:57
@blt blt force-pushed the blt/slim_out_joinset branch from 38e96c1 to 6055d12 Compare December 6, 2024 03:12
@blt blt force-pushed the blt/total_rss_bytes_and_total_pss_bytes_from_smaps_rollup branch from 84cd41c to 0eea62b Compare December 6, 2024 03:13
Copy link
Contributor

@GeorgeHahn GeorgeHahn left a comment

Choose a reason for hiding this comment

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

Change LGTM. Will this change the RSS measurement enough that we'll need to update limits to account for it?

@blt blt force-pushed the blt/slim_out_joinset branch from 6055d12 to c5ff9b3 Compare December 6, 2024 15:42
@blt blt force-pushed the blt/total_rss_bytes_and_total_pss_bytes_from_smaps_rollup branch from 0eea62b to ce92ec6 Compare December 6, 2024 15:43
@blt
Copy link
Collaborator Author

blt commented Dec 6, 2024

Change LGTM. Will this change the RSS measurement enough that we'll need to update limits to account for it?

My expectation is that these measurements will be basically the same, potentially slightly larger. smaps data elides < 1Kb because of the way it rounds, so if a target has a lot of little regions that'll matter.

Will need to be checked.

@blt blt force-pushed the blt/slim_out_joinset branch from c5ff9b3 to b91521a Compare December 6, 2024 16:18
@blt blt force-pushed the blt/total_rss_bytes_and_total_pss_bytes_from_smaps_rollup branch from ce92ec6 to ed0d2aa Compare December 6, 2024 16:19
@blt blt force-pushed the blt/slim_out_joinset branch from b91521a to d304542 Compare December 6, 2024 16:52
@blt blt force-pushed the blt/total_rss_bytes_and_total_pss_bytes_from_smaps_rollup branch from ed0d2aa to c9ba26b Compare December 6, 2024 16:53
@blt blt force-pushed the blt/slim_out_joinset branch from d304542 to 9f7428e Compare December 6, 2024 17:28
@blt blt force-pushed the blt/total_rss_bytes_and_total_pss_bytes_from_smaps_rollup branch from c9ba26b to f781927 Compare December 6, 2024 17:28
@blt blt force-pushed the blt/slim_out_joinset branch from 9f7428e to f38ed48 Compare December 6, 2024 17:38
@blt blt force-pushed the blt/total_rss_bytes_and_total_pss_bytes_from_smaps_rollup branch from f781927 to 9bebbd0 Compare December 6, 2024 17:38
@blt blt force-pushed the blt/slim_out_joinset branch from f38ed48 to 15b75e4 Compare December 6, 2024 17:39
@blt blt force-pushed the blt/total_rss_bytes_and_total_pss_bytes_from_smaps_rollup branch from 9bebbd0 to 37274b0 Compare December 6, 2024 17:40
@blt blt force-pushed the blt/slim_out_joinset branch from 15b75e4 to 06d7bab Compare December 6, 2024 17:47
@blt blt force-pushed the blt/total_rss_bytes_and_total_pss_bytes_from_smaps_rollup branch from 37274b0 to 3079584 Compare December 6, 2024 17:47
@blt blt force-pushed the blt/slim_out_joinset branch from 06d7bab to 42af96f Compare December 6, 2024 17:51
@blt blt force-pushed the blt/total_rss_bytes_and_total_pss_bytes_from_smaps_rollup branch from 3079584 to b42be3f Compare December 6, 2024 17:51
@scottopell
Copy link
Contributor

We should deprecate rss_bytes (similar to total_rss_bytes but per-process) in favor of smaps_rollup.rss. OR define rss_bytes from smaps_rollup.rss

@blt blt force-pushed the blt/slim_out_joinset branch from 42af96f to e7ffc27 Compare December 6, 2024 18:14
@blt blt force-pushed the blt/total_rss_bytes_and_total_pss_bytes_from_smaps_rollup branch from b42be3f to ce4fe14 Compare December 6, 2024 18:15
@blt blt force-pushed the blt/slim_out_joinset branch 2 times, most recently from e26e932 to 88a426e Compare December 6, 2024 19:24
@blt blt force-pushed the blt/total_rss_bytes_and_total_pss_bytes_from_smaps_rollup branch from ce4fe14 to b64cb17 Compare December 6, 2024 19:24
@blt blt changed the base branch from blt/slim_out_joinset to graphite-base/1134 December 6, 2024 19:58
@blt blt force-pushed the blt/total_rss_bytes_and_total_pss_bytes_from_smaps_rollup branch from b64cb17 to 1bbbca7 Compare December 6, 2024 19:58
@blt blt force-pushed the graphite-base/1134 branch from 88a426e to d3306ef Compare December 6, 2024 19:58
@blt blt changed the base branch from graphite-base/1134 to main December 6, 2024 19:59
This commit adds collection of total PSS bytes for all processes from
smaps_rollup, adjusting total_rss_bytes to also be from the same source.

Signed-off-by: Brian L. Troutwine <[email protected]>
@blt blt force-pushed the blt/total_rss_bytes_and_total_pss_bytes_from_smaps_rollup branch from 1bbbca7 to 0ed3531 Compare December 6, 2024 19:59
@blt blt merged commit dce08ac into main Dec 6, 2024
18 checks passed
Copy link
Collaborator Author

blt commented Dec 6, 2024

Merge activity

  • Dec 6, 3:33 PM EST: A user merged this pull request with Graphite.

@blt blt deleted the blt/total_rss_bytes_and_total_pss_bytes_from_smaps_rollup branch December 6, 2024 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants