Use /proc/PID/smaps_rollup for the resident-unique reporter on Linux
Categories
(Core :: XPCOM, task)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox157 | --- | fixed |
People
(Reporter: jstutte, Assigned: perdomojuan755, Mentored)
References
Details
(Keywords: good-first-bug, perf-alert, Whiteboard: [lang=c++])
Attachments
(1 file)
|
Bug 2069230 - Use smaps_rollup for resident-unique reporter. r?#xpcom-reviewers!,#sandbox-reviewers!
48 bytes,
text/x-phabricator-request
|
Details | Review |
Filing as a good first bug to learn workflows.
On Linux, the "resident-unique" memory reporter (USS) is computed by parsing every entry of /proc/PID/smaps and summing Private_Clean + Private_Dirty across all mappings. The kernel already exposes those two numbers, pre-summed, in /proc/PID/smaps_rollup: a single block of about 700 bytes, instead of one 25-line block per mapping.
The change is to make ResidentUniqueDistinguishedAmount() read smaps_rollup directly (honouring its aPid argument, so it still works for other processes), and fall back to the existing GetProcSelfSmapsPrivate() path when smaps_rollup cannot be opened. The fallback is needed because smaps_rollup was added in Linux 4.14 and Firefox still supports older kernels.
Please do not change GetMemoryMappings() itself. Its other caller, the "memory-mappings" reporter, needs the full per-mapping detail and must keep parsing smaps.
Link to the code:
ResidentUniqueDistinguishedAmount(), the function to change:
https://searchfox.org/firefox-main/rev/298cf786229591e2159a3933605df11e07709d6f/xpcom/base/nsMemoryReporterManager.cpp#123
GetProcSelfSmapsPrivate(), the current implementation and the fallback:
https://searchfox.org/firefox-main/rev/298cf786229591e2159a3933605df11e07709d6f/xpcom/base/nsMemoryReporterManager.cpp#90
GetMemoryMappings(), the full smaps parser it calls:
https://searchfox.org/firefox-main/rev/298cf786229591e2159a3933605df11e07709d6f/xpcom/base/MemoryMapping.cpp#105
To verify the fix:
There is no unit test covering this value, so the check is that the reported number does not change.
-
Pick any running process and confirm the two sources agree:
awk '/^Private_Clean:/{c+=$2} /^Private_Dirty:/{d+=$2} END{print c+d}' /proc/<pid>/smaps
awk '/^Private_(Clean|Dirty):/{t+=$2} END{print t}' /proc/<pid>/smaps_rollupBoth must print the same number of kB.
-
Build with ./mach build, then open about:memory and click "Measure". The "resident-unique" figure reported for each process must match what an unpatched build reports for a comparable session, and must match the awk numbers above for the corresponding pid.
Why this is worth doing: measured on a Firefox parent process with roughly 3000 mappings, parsing the full smaps costs about 10 ms per call. Around half of that is kernel time spent formatting about 2 MB of text, and around half is our own line-by-line parsing (one std::getline per line, plus an sscanf or strtok_r and a table lookup for each). Reading smaps_rollup costs about 3 ms. The page-table walk the kernel performs is identical either way, so roughly 7 ms of the 10 ms is avoidable.
Tutorial to contribute:
https://firefox-source-docs.mozilla.org/contributing/contribution_quickref.html
https://firefox-source-docs.mozilla.org/contributing/stack_quickref.html
Please don't ask for the bug to be assigned. It will be automatically assigned to the first patch.
| Assignee | ||
Comment 1•27 days ago
|
||
Updated•27 days ago
|
Updated•27 days ago
|
Updated•27 days ago
|
Comment 3•22 days ago
|
||
| bugherder | ||
Comment 4•17 days ago
|
||
Perfherder has detected a mozperftest performance change from push 33574b7c70b2327d3ea44aeaff02f8b6d3c320d0.
No action is required from the author; this comment is provided for informational purposes only.
| Improvements | Test | Platform | Options | Absolute values [old vs new] |
|---|---|---|---|---|
| 69% | background-resource cpuTime-tab-background-diff | android-hw-a55-14-0-aarch64-shippable | 505.00 ms -> 156.67 ms | |
| 60% | background-resource cpuTime-tab-backgrounding-diff | android-hw-a55-14-0-aarch64-shippable | 305.83 ms -> 121.67 ms | |
| 28% | background-resource cpuTime-tab-50% | android-hw-a55-14-0-aarch64-shippable | 1,219.17 ms -> 873.33 ms | |
| 28% | background-resource cpuTime-tab-end | android-hw-a55-14-0-aarch64-shippable | 1,219.17 ms -> 879.17 ms | |
| 28% | foreground-resource cpuTime-tab-end | android-hw-a55-14-0-aarch64-shippable | 1,244.58 ms -> 901.67 ms | |
| ... | ... | ... | ... | ... |
| 16% | foreground-resource cpuTime-tab-10% | android-hw-a55-14-0-aarch64-shippable | 1,044.17 ms -> 874.17 ms |
Need Help or Information?
If you have any questions, please reach out to fbilt@mozilla.com. Alternatively, you can find help on Slack by joining #perf-help, and on Matrix you can find help by joining #perftest.
Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests.
| Reporter | ||
Comment 5•17 days ago
|
||
Surprisingly this perf improvement is real, apparently. Note that on Android however we do not gather memory telemetry (among others) on release, so end users will feel this less (but there is a smaller improvement for Linux, too, and that rides to release).
Description
•