Report memory regions by status bits

RESOLVED FIXED in Firefox 37

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: dmajor, Assigned: njn)

Tracking

unspecified
mozilla39
x86_64
Windows 7
Points:
---

Firefox Tracking Flags

(firefox37 fixed, firefox38 fixed, firefox39 fixed)

Details

(Whiteboard: [MemShrink])

Attachments

(2 attachments)

Reporter

Description

4 years ago
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
Reporter

Comment 2

4 years ago
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.
Reporter

Comment 3

4 years ago
As an additional project we might also want to take the "Tiny" and "Misaligned" numbers from attachment 8428481 [details] [diff] [review] (of bug 1001760).
Reporter

Comment 4

4 years ago
Nick, could your team take it from here? Just need to take the logic from the patch and make it about-memory-esque.
Flags: needinfo?(n.nethercote)
Assignee

Updated

4 years ago
Assignee: nobody → n.nethercote
Flags: needinfo?(n.nethercote)
Assignee

Comment 5

4 years ago
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.
Attachment #8566924 - Flags: review?(dmajor)
Assignee

Comment 6

4 years ago
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.
Reporter

Comment 7

4 years ago
> glandium pointed out that "free" will dominate on 64-bit...
Amusing :) but IMO it's not really a problem.
Reporter

Comment 8

4 years ago
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?
Attachment #8566924 - Flags: review?(dmajor) → review+
https://hg.mozilla.org/mozilla-central/rev/e0e55dfe6955
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
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.
Attachment #8566924 - Flags: approval-mozilla-beta?
Attachment #8566924 - Flags: approval-mozilla-aurora?
Attachment #8566924 - Flags: approval-mozilla-beta?
Attachment #8566924 - Flags: approval-mozilla-beta+
Attachment #8566924 - Flags: approval-mozilla-aurora?
Attachment #8566924 - Flags: approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.