Closed Bug 1311039 Opened 3 years ago Closed 3 years ago

about:memory gives a negative response in some areas due to 'heap-allocated' being zero

Categories

(Core :: Memory Allocator, defect, P1)

50 Branch
defect

Tracking

()

VERIFIED FIXED
mozilla53
Tracking Status
firefox53 --- verified

People

(Reporter: docklandser, Assigned: glandium, NeedInfo)

References

(Blocks 1 open bug)

Details

(Whiteboard: [MemShrink:P1])

Attachments

(2 files)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:50.0) Gecko/20100101 Firefox/50.0
Build ID: 20161017130949

Steps to reproduce:

Just type in about:memory, v simple! Tried it in profile and in blank new profile.


Actual results:

Main Process

WARNING: the following values are negative or unreasonably large.

    explicit/js-non-window
    explicit/(12 tiny)
    explicit/(12 tiny)/heap-unclassified 

This indicates a defect in one or more memory reporters. The invalid values are highlighted. 

I'll attach the verbose memory report.


Expected results:

Non-negative- as Firefox said its a bug, then I assume it's a bug. And I'm trying to be community minded and report it like a good user should. It seems a bit, er, difficult, that last part, to be frank, but I love the hell out of your browser, have customized it a bit (all just appearance, nowt weird) and couldn't/wouldn't use anything else. You have me as a customer til you stop.
Product: Firefox → Core
Whiteboard: [MemShrink]
I feel like I've seen a very similar one to this recently, but I don't remember what it is.

The totals for explicit/js-non-window and explicit/window-objects are larger than explicit! There are also a large number of JS zones. My only theory here is that somehow sharing of shapes isn't being properly handled in the memory reporter, but I'd assume somebody else would have run DMD in the mean time and noticed that.
Status: UNCONFIRMED → NEW
Component: Untriaged → JavaScript Engine
Ever confirmed: true
Well, bug 1295967 didn't land until 51, and it sounds like the reporter is using 50. (At least based on comment 0.)
This is pretty odd, looks like mozjemalloc isn't reporting it's memory?

>    0.00 MB ── heap-allocated
>    1.00 MB ── heap-chunksize
>    1.00 MB ── heap-mapped

I'm reasonably sure we're doing heap-unclassified = heap-allocated - sum(explicit-tree). Nick does that sound about right?
Flags: needinfo?(n.nethercote)
(In reply to Eric Rahm [:erahm] from comment #3)
> This is pretty odd, looks like mozjemalloc isn't reporting it's memory?
> 
> >    0.00 MB ── heap-allocated
> >    1.00 MB ── heap-chunksize
> >    1.00 MB ── heap-mapped

This is the root of the problem. I don't why heap-allocated would be zero, I don't think I've seen that before.
 
> I'm reasonably sure we're doing heap-unclassified = heap-allocated -
> sum(explicit-tree). Nick does that sound about right?

Close. It's this: heap-unclassified = heap-allocated - sum(all explicit measurements with kind == HEAP)

So heap-allocated is bogus, which makes heap-unclassified bogus, which makes explicit bogus, which leads to complaints about js-non-window and window-objects.
Flags: needinfo?(n.nethercote)
Summary: about:memory gives a negative response in some areas → about:memory gives a negative response in some areas due to 'heap-allocated' being zero
Duplicate of this bug: 1312311
I'm going to P3 this for now as it's not an actual memory leak. ni'ing myself to test on OSX with an official release build (not nightly) to see if there's something wonky with jemalloc on 10.12 + release builds.
Flags: needinfo?(erahm)
Whiteboard: [MemShrink] → [MemShrink:P3]
Essentially, this is a different incarnation of bug 1284677. Nightly is not affected because it uses replace-malloc, which uses a different (and now fixed) zone registration. Replace-malloc is not enabled on non-nightly builds.

Mozjemalloc needs to be adjusted such that the zone is properly registered on 10.12.  For some reason, I've always avoided touching the mozjemalloc zone registration code, maybe it's time to just make it use the same as replace-malloc and upstream jemalloc.
Okay, bumping the priority and moving to memory allocator. Not using (moz)jemalloc on 10.12 seems bad.
Component: JavaScript Engine → Memory Allocator
Flags: needinfo?(erahm)
Whiteboard: [MemShrink:P3] → [MemShrink:P1]
Assignee: nobody → mh+mozilla
Can someone test this try build on macOS 10.12 and see if heap-allocated is non-zero?
https://archive.mozilla.org/pub/firefox/try-builds/mh@glandium.org-af5735651877ce104e495140e8a84cacdf2440b8/try-macosx64/

(Also, if someone could test that it still works on older versions...)
Please see comment 9.
Flags: needinfo?(erahm)
Flags: needinfo?(docklandser)
Priority: -- → P1
Comment on attachment 8810730 [details]
Bug 1311039 - Properly detect the default malloc zone on OSX 10.12.

https://reviewboard.mozilla.org/r/93044/#review93000

rs=me
Attachment #8810730 - Flags: review?(n.nethercote) → review+
(In reply to Mike Hommey [:glandium] from comment #9)
> Can someone test this try build on macOS 10.12 and see if heap-allocated is
> non-zero?
> https://archive.mozilla.org/pub/firefox/try-builds/mh@glandium.org-
> af5735651877ce104e495140e8a84cacdf2440b8/try-macosx64/

Confirmed, heap-allocated is non-zero.

> (Also, if someone could test that it still works on older versions...)

I can only confirm 10.12, but presumably our try servers can confirm 10.10. I'd suggest adding/updating a unit test that makes sure heap-allocated is always non-zero.
Flags: needinfo?(erahm)
As per irc, and further validation on try, there already is a unit test that makes sure heap-allocated is non-zero. Except it doesn't run on 10.12, so it hasn't detected *this* bug. (But, if I force jemalloc not to be hooked on all osx versions, it correctly detects that)
Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/autoland/rev/2e0ae44e91ae
Properly detect the default malloc zone on OSX 10.12. r=njn
https://hg.mozilla.org/mozilla-central/rev/2e0ae44e91ae
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Let's make sure this works as intended on Mac OS X 10.9 to macOS 10.12. Instructions in Comment 0.
Flags: qe-verify+
I could not reproduce the issue with instructions from description. Can you please give me more details?

Thank you!
(In reply to Cristian Comorasu [:CristiComo] from comment #19)
> I could not reproduce the issue with instructions from description. Can you
> please give me more details?
> 
> Thank you!

What did you try?
I followed the steps from the description using the nightly build from the date this bug was filled and compared the results with the latest beta. What am I missing?
Thank you for the quick reply.
Flags: needinfo?(mh+mozilla)
(In reply to Cristian Comorasu [:CristiComo] from comment #21)
> I followed the steps from the description using the nightly build from the
> date this bug was filled and compared the results with the latest beta. What
> am I missing?
> Thank you for the quick reply.

Did you try on OSX 10.12?
Flags: needinfo?(mh+mozilla)
Thank you Mike Hommey for clarifying via irc!

I reproduced this issue using Fx 52.0b1, on Mac OS X 10.12.3. 
I can confirm this issue is fixed, I verified using Fx 53.0b3.

The following platforms were verified:
 - Mac OS X 10.12.3
 - Mac OS X 10.11.6
 - Mac OS X 10.10.5
 - Mac OS X 10.9.5

Cheers!
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.