Closed Bug 2069230 Opened 28 days ago Closed 22 days ago

Use /proc/PID/smaps_rollup for the resident-unique reporter on Linux

Categories

(Core :: XPCOM, task)

task

Tracking

()

RESOLVED FIXED
157 Branch
Tracking Status
firefox157 --- fixed

People

(Reporter: jstutte, Assigned: perdomojuan755, Mentored)

References

Details

(Keywords: good-first-bug, perf-alert, Whiteboard: [lang=c++])

Attachments

(1 file)

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.

  1. 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_rollup

    Both must print the same number of kB.

  2. 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: nobody → perdomojuan755
Status: NEW → ASSIGNED
Attachment #9636964 - Attachment description: Bug 2069230 - Use smaps_rollup for resident-unique reporter. r?#xpcom-reviewers → Bug 2069230 - Use smaps_rollup for resident-unique reporter. r?#xpcom-reviewers,#sandbox-reviewers!
Attachment #9636964 - Attachment description: Bug 2069230 - Use smaps_rollup for resident-unique reporter. r?#xpcom-reviewers,#sandbox-reviewers! → Bug 2069230 - Use smaps_rollup for resident-unique reporter. r?#xpcom-reviewers!,#sandbox-reviewers!
Pushed by ealvarez@mozilla.com: https://github.com/mozilla-firefox/firefox/commit/2537dfab8f3f https://hg.mozilla.org/integration/autoland/rev/33574b7c70b2 Use smaps_rollup for resident-unique reporter. r=xpcom-reviewers,sandbox-reviewers,jld,emilio
Status: ASSIGNED → RESOLVED
Closed: 22 days ago
Resolution: --- → FIXED
Target Milestone: --- → 157 Branch

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.

Keywords: perf-alert
See Also: → 2072217

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).

You need to log in before you can comment on or make changes to this bug.