Last Comment Bug 1134030 - Report memory regions by status bits
: Report memory regions by status bits
Status: RESOLVED FIXED
[MemShrink]
:
Product: Toolkit
Classification: Components
Component: about:memory (show other bugs)
: unspecified
: x86_64 Windows 7
-- normal (vote)
: mozilla39
Assigned To: Nicholas Nethercote [:njn]
:
: Nicholas Nethercote [:njn]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2015-02-17 15:07 PST by David Major [:dmajor]
Modified: 2015-03-11 11:32 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
fixed
fixed


Attachments
patch on top of minidump-memorylist (4.86 KB, patch)
2015-02-17 15:08 PST, David Major [:dmajor]
no flags Details | Diff | Splinter Review
Add WindowsAddressSpaceReporter. code=njn,dmajor (5.90 KB, patch)
2015-02-19 18:18 PST, Nicholas Nethercote [:njn]
dmajor: review+
lukasblakk+bugs: approval‑mozilla‑aurora+
lukasblakk+bugs: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description User image David Major [:dmajor] 2015-02-17 15:07:59 PST
I have a modified version of breakpad's minidump-memorylist tool that breaks down address space by the page status and protection bits. It has been very useful for issues like bug 1062065 and bug 1128999.

We can build this into about:memory pretty easily. The memory reporters already do an address space walk for the vsize-max-contiguous calculation: https://dxr.mozilla.org/mozilla-central/source/xpcom/base/nsMemoryReporterManager.cpp#509
Comment 1 User image David Major [:dmajor] 2015-02-17 15:08:44 PST
Created attachment 8565708 [details] [diff] [review]
patch on top of minidump-memorylist
Comment 2 User image David Major [:dmajor] 2015-02-17 15:11:43 PST
So far RESV and GFX have been the most useful. FREE and HEAP are probably covered by existing reporters. The remaining categories may or may not be useful someday.
Comment 3 User image David Major [:dmajor] 2015-02-17 15:13:24 PST
As an additional project we might also want to take the "Tiny" and "Misaligned" numbers from attachment 8428481 [details] [diff] [review] (of bug 1001760).
Comment 4 User image David Major [:dmajor] 2015-02-17 15:17:45 PST
Nick, could your team take it from here? Just need to take the logic from the patch and make it about-memory-esque.
Comment 5 User image Nicholas Nethercote [:njn] 2015-02-19 18:18:46 PST
Created attachment 8566924 [details] [diff] [review]
Add WindowsAddressSpaceReporter. code=njn,dmajor

Sample output:

> 2,147,418,112 B (100.0%) -- address-space
> +--1,662,676,992 B (77.43%) -- free [229]
> +----299,868,160 B (13.96%) -- commit
> |    +--142,098,432 B (06.62%) -- image
> |    |  +---94,035,968 B (04.38%) -- execute-read [107]
> |    |  +---44,826,624 B (02.09%) -- readonly [245]
> |    |  +----1,736,704 B (00.08%) -- writecopy [62]
> |    |  +----1,499,136 B (00.07%) -- readwrite [151]
> |    +--127,987,712 B (05.96%) -- private
> |    |  +--122,867,712 B (05.72%) -- readwrite [224]
> |    |  +----2,977,792 B (00.14%) -- readwrite (stack) [128]
> |    |  +----1,310,720 B (00.06%) -- readwrite+guard [128]
> |    |  +------729,088 B (00.03%) -- execute-readwrite [13]
> |    |  +-------53,248 B (00.00%) -- execute-read [11]
> |    |  +-------49,152 B (00.00%) -- readonly [12]
> |    +---29,782,016 B (01.39%) -- mapped
> |        +--18,513,920 B (00.86%) -- readwrite [31]
> |        +--11,268,096 B (00.52%) -- readonly [25]
> +----184,872,960 B (08.61%) -- reserved
>      +--161,124,352 B (07.50%) -- private [177]
>      +---23,474,176 B (01.09%) -- mapped [5]
>      +------274,432 B (00.01%) -- image [6]

The numbers in square brackets indicate how many ranges had those attributes.
It's reassuring that the number of "readwrite (stack)" ranges matches the
number of "readwrite+guard" ranges. It's also reassuring that the total is a
smidge under 2 GiB.
Comment 6 User image Nicholas Nethercote [:njn] 2015-02-19 19:39:28 PST
glandium pointed out that "free" will dominate on 64-bit...

> 134,217,727.94 MB (100.0%) -- address-space
> +--134,217,127.41 MB (100.0%) -- free [208]
> +----------600.53 MB (00.00%) ++ (2 tiny)

128 TiB! dmajor, any thoughts about this? Maybe it's not a problem.
Comment 7 User image David Major [:dmajor] 2015-02-22 14:03:27 PST
> glandium pointed out that "free" will dominate on 64-bit...
Amusing :) but IMO it's not really a problem.
Comment 8 User image David Major [:dmajor] 2015-02-22 14:16:53 PST
Comment on attachment 8566924 [details] [diff] [review]
Add WindowsAddressSpaceReporter. code=njn,dmajor

Review of attachment 8566924 [details] [diff] [review]:
-----------------------------------------------------------------

I really like this approach! Thanks for working on this.

::: xpcom/base/nsMemoryReporterManager.cpp
@@ +680,5 @@
> +      if (NS_WARN_IF(NS_FAILED(rv))) {
> +        return rv;
> +      }
> +
> +      SIZE_T lastAddress = currentAddress;

currentAddress is a lowercase size_t so I guess lastAddress can be lowercase too.

@@ +689,5 @@
> +        break;
> +      }
> +    }
> +
> +#undef REPORT

What's this for?
Comment 9 User image Nicholas Nethercote [:njn] 2015-02-22 18:08:28 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/e0e55dfe6955
Comment 10 User image Wes Kocher (:KWierso) 2015-02-23 16:08:13 PST
https://hg.mozilla.org/mozilla-central/rev/e0e55dfe6955
Comment 11 User image Ralph Giles (:rillian) | needinfo me 2015-03-10 17:01:41 PDT
Comment on attachment 8566924 [details] [diff] [review]
Add WindowsAddressSpaceReporter. code=njn,dmajor

Requesting uplift to 38 and 37. We think this might help us diagnose remaining reported memory issues with the MSE feature.

Approval Request Comment
[Feature/regressing bug #]: MSE
[User impact if declined]: weaker testing, less information available during bug reproduction.
[Describe test coverage new/current, TreeHerder]: Landed on m-c.
[Risks and why]: This only affects about:memory and has had two weeks to settle on m-c, so risk should be acceptable.
[String/UUID change made/needed]: None.
Comment 12 User image Ralph Giles (:rillian) | needinfo me 2015-03-11 11:26:18 PDT
https://hg.mozilla.org/releases/mozilla-beta/rev/20306323469e
Comment 13 User image Ralph Giles (:rillian) | needinfo me 2015-03-11 11:32:18 PDT
https://hg.mozilla.org/releases/mozilla-aurora/rev/04c022de147f

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