benchmark_perf: report PSS alongside summed RSS - #55
Open
Rian354 wants to merge 1 commit into
Open
Conversation
Every harness peaked on parent RSS plus the summed RSS of all children. That double-counts copy-on-write pages shared between forked workers, and the error is not small: a 64-worker pandarallel ingest measured this way summed to 1.89 TB on a 480 GB node. The same accounting inflated our own PyHealth 2.0 worker sweep, where PSS shows 32.1 GB at 32 workers against 39.1 GB for summed RSS. Adds PSS (proportional set size -- each shared page divided among the processes mapping it) across the process tree, with an RSS fallback where PSS is unavailable. Additive: peak_rss_bytes keeps its existing meaning so numbers already collected stay interpretable, and peak_pss_bytes lands beside it. Applies to all 16 harnesses: legacy, workers_n, meds_reader, and patient_exploration variants.
This file contains hidden or 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
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.
Every harness in
examples/benchmark_perf/measured peak memory as parent RSS plus the summed RSS of all children:That double-counts copy-on-write pages shared between forked workers, and the error is not small. Measuring a 64-worker pandarallel ingest this way produced 1.89 TB on a 480 GB node — physically impossible, which is how the problem surfaced. The same accounting inflated our own PyHealth 2.0 worker sweep:
PSS (proportional set size) divides each shared page among the processes mapping it, so the total is additive. It is the figure to quote when the question is "how much memory does this need."
What changed
Adds
_total_pss_bytes()and apeak_pss_bytesfield to all 16 harnesses — legacy,workers_n,meds_reader, andpatient_explorationvariants. Falls back to RSS per-process where PSS is unavailable (non-Linux, or insufficient permissions).Additive on purpose.
peak_rss_byteskeeps its existing meaning so previously-collected numbers remain interpretable; PSS lands beside it rather than silently replacing it. Summed RSS is also worth keeping as a negative control — the gap between the two columns is what demonstrates the sharing.Not addressed here:
torch.cuda.max_memory_allocatedsimilarly understates GPU requirements (a cell reporting 5.9 GB allocated held 10.19 GiB and OOMed on a 10.57 GiB card), but that is a separate change intrainer.py.