Closed
Bug 1145007
Opened 9 years ago
Closed 9 years ago
Add "resident-peak" memory measurement on Unix
Categories
(Core :: XPCOM, defect)
Tracking
()
RESOLVED
FIXED
mozilla39
Tracking | Status | |
---|---|---|
firefox39 | --- | fixed |
People
(Reporter: n.nethercote, Assigned: n.nethercote)
References
Details
(Whiteboard: [MemShrink])
Attachments
(2 files, 2 obsolete files)
1.16 KB,
patch
|
erahm
:
review+
|
Details | Diff | Splinter Review |
9.42 KB,
patch
|
erahm
:
review+
|
Details | Diff | Splinter Review |
This has been suggested several times recently. It can be read from the "VmHWM" entry in /proc/<pid>/status.
Assignee | ||
Comment 1•9 years ago
|
||
It is the only distinguished amount in nsIMemoryReporterManager that isn't being tested.
Attachment #8579805 -
Flags: review?(erahm)
Assignee | ||
Comment 2•9 years ago
|
||
Attachment #8579806 -
Flags: review?(erahm)
Comment 3•9 years ago
|
||
Doesn't getrusage give the same value in ru_maxrss?
Assignee | ||
Comment 4•9 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #3) > Doesn't getrusage give the same value in ru_maxrss? Oh, I didn't know about getrusage. It gives the same result and is easier to use, so I'll switch.
Assignee | ||
Updated•9 years ago
|
Attachment #8579806 -
Attachment is obsolete: true
Attachment #8579806 -
Flags: review?(erahm)
Comment 6•9 years ago
|
||
Note that getrusage is POSIX, you can also use it on OSX as well (or even #ifdef XP_UNIX, likely)
Comment 7•9 years ago
|
||
Comment on attachment 8579805 [details] [diff] [review] (part 1) - Test |residentUnique| in test_memoryReporters.xul Review of attachment 8579805 [details] [diff] [review]: ----------------------------------------------------------------- r=me
Attachment #8579805 -
Flags: review?(erahm) → review+
Comment 8•9 years ago
|
||
Comment on attachment 8579838 [details] [diff] [review] (part 2) - Add a "resident-peak" distinguished amount and memory reporter on Linux Review of attachment 8579838 [details] [diff] [review]: ----------------------------------------------------------------- lgtm. It would be nice to have this supported on OSX/BSD as well. Can you file a follow up for that?
Attachment #8579838 -
Flags: review?(erahm) → review+
Assignee | ||
Comment 9•9 years ago
|
||
> It would be nice to have this supported on OSX/BSD as well. Can you
> file a follow up for that?
I'll just do it in this patch, by moving the peak stuff in nsMemoryReporterManager.cpp into a XP_UNIX block.
Assignee | ||
Updated•9 years ago
|
Summary: Add "resident-peak" memory measurement on Linux → Add "resident-peak" memory measurement on Unix
Assignee | ||
Comment 10•9 years ago
|
||
ResidentPeakDistinguishedAmount() is now in the XP_UNIX section, and I'm handling the different units for different Unices, and I'm handling the possibility of 0, in case Solaris returns that.
Attachment #8580322 -
Flags: review?(erahm)
Assignee | ||
Updated•9 years ago
|
Attachment #8579838 -
Attachment is obsolete: true
Comment 11•9 years ago
|
||
Comment on attachment 8580322 [details] [diff] [review] (part 2) - Add a "resident-peak" distinguished amount and memory reporter on Unix Review of attachment 8580322 [details] [diff] [review]: ----------------------------------------------------------------- r=me, one super minor suggestion. ::: xpcom/base/nsMemoryReporterManager.cpp @@ +810,5 @@ > +ResidentPeakDistinguishedAmount(int64_t* aN) > +{ > + struct rusage usage; > + if (0 == getrusage(RUSAGE_SELF, &usage)) { > + // The units for ru_maxrrs: nit: ru_maxrss @@ +1923,5 @@ > + > +/*static*/ int64_t > +nsMemoryReporterManager::ResidentPeak() > +{ > +#ifdef HAVE_RESIDENT_PEAK_REPORTER min--: This could just call GetResidentPeak and drop the #ifdefs.
Attachment #8580322 -
Flags: review?(erahm) → review+
Assignee | ||
Comment 12•9 years ago
|
||
> > +/*static*/ int64_t
> > +nsMemoryReporterManager::ResidentPeak()
> > +{
> > +#ifdef HAVE_RESIDENT_PEAK_REPORTER
>
> min--: This could just call GetResidentPeak and drop the #ifdefs.
I thought that too, but this one is static and GetResidentPeak() is not.
Assignee | ||
Comment 13•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/228183a22063 https://hg.mozilla.org/integration/mozilla-inbound/rev/3ecc82272861
Comment 14•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/228183a22063 https://hg.mozilla.org/mozilla-central/rev/3ecc82272861
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox39:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
You need to log in
before you can comment on or make changes to this bug.
Description
•