Open
Bug 1476168
Opened 6 years ago
Updated 6 months ago
Update /proc/self/smaps consumers to use MemoryMapping.h, and parse once per memory reporter cycle
Categories
(Core :: XPCOM, enhancement)
Core
XPCOM
Tracking
()
NEW
People
(Reporter: kmag, Unassigned)
Details
Attachments
(1 file)
There are currently a bunch of places where we read /proc/self/smaps or /maps that should probably be updated to use the MemoryMapping.h helper instead. More importantly, we have two separate places where we read /proc/self/smaps during each memory reporting cycle, and may need more in the future. We should really have a helper to read and parse it once per cycle, and allow different reporters to use the cached data as necessary.
Comment hidden (mozreview-request) |
Comment 2•6 years ago
|
||
mozreview-review |
Comment on attachment 8994377 [details] Bug 1476168: Only parse /proc/self/smaps once per memory reporter cycle. https://reviewboard.mozilla.org/r/258950/#review266490 ::: xpcom/base/nsMemoryReporterManager.cpp:109 (Diff revision 1) > // statm's "shared" value actually counts pages backed by files, which has > // little to do with whether the pages are actually shared. /proc/self/smaps > // on the other hand appears to give us the correct information. > > - nsTArray<MemoryMapping> mappings(1024); > - MOZ_TRY(GetMemoryMappings(mappings)); > + auto* mgr = nsMemoryReporterManager::GetOrCreate(); > + const auto& mappings = mgr->GetMemoryMappings(); This isn't quite right. You can query this value directly outside of a `CollectReports` call via [nsIMemoryReporterManager.residentUnique](https://searchfox.org/mozilla-central/source/xpcom/base/nsIMemoryReporter.idl#388). Multiple calls would all return the same cached value. What I was envisioning is merging the `ThreadsReporter` and the `ResidentUniqueReporters` (at least on Linux), and then we could pass in `mappings` to `GetProcSelfSmapsPrivate`. ::: xpcom/base/nsMemoryReporterManager.cpp:1850 (Diff revision 1) > // be called from the main thread. > if (!NS_IsMainThread()) { > MOZ_CRASH(); > } > > + ClearMemoryMappings(); Having to add all these in is pretty tedious and error prone. In this case we should not clear if `mPendingProcessesState`.
Attachment #8994377 -
Flags: review?(erahm) → review-
Reporter | ||
Comment 3•6 years ago
|
||
mozreview-review-reply |
Comment on attachment 8994377 [details] Bug 1476168: Only parse /proc/self/smaps once per memory reporter cycle. https://reviewboard.mozilla.org/r/258950/#review266490 > This isn't quite right. You can query this value directly outside of a `CollectReports` call via [nsIMemoryReporterManager.residentUnique](https://searchfox.org/mozilla-central/source/xpcom/base/nsIMemoryReporter.idl#388). Multiple calls would all return the same cached value. > > What I was envisioning is merging the `ThreadsReporter` and the `ResidentUniqueReporters` (at least on Linux), and then we could pass in `mappings` to `GetProcSelfSmapsPrivate`. Yeah, I realized that when I was looking at bug 1477662. :/
Updated•2 years ago
|
Severity: normal → S3
Comment 4•6 months ago
|
||
The bug assignee is inactive on Bugzilla, so the assignee is being reset.
Assignee: kmaglione+bmo → nobody
You need to log in
before you can comment on or make changes to this bug.
Description
•