704391 - Add more JS runtime memory reporters and fix the existing ones

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Nicholas Nethercote [:njn] (On Long Service Leave)]

Attachment: patch

This patch adds some new memory reporters:

- js/runtime/temporary, for JSThreadData::tempLifoAlloc.  This is usually a
  few 100s of KB, but sometimes it spikes to multiple MBs due to parse
  nodes, and so can reduce heap-unclassified by a decent amount.
  
- js/runtime/thread-objects, for JSThreads.  On 64-bit it's usually about
  135KB or 270KB depending on whether there are one or two JSThreads.

- js/runtime/contexts, for JSContexts.  This is usually pretty small, eg.
  10s of KB.

Also, two existing reporters, js/runtime/runtime-object and
js/runtime/atoms-table, were incorrectly marked as being non-heap memory.  
This means that both "explicit" and "heap-unclassified" were over-reporting
by the sum of these two measurements, which is often about 4.5MB.  The patch
fixes this.  (I also audited all our NONHEAP-marked reporters, all the 
others were correct.)

Comment 1

[Nicholas Nethercote [:njn] (On Long Service Leave)]

Attachment: patch v2

This version includes JSContext::busyArrays in js/runtime/contexts.  It turns out the busyArrays are typically 2 to 3x bigger than the JSContext objects themselves.  (Nb: the |sizeOfIncludingThis| naming convention is something I'm introducing concurrently in bug 698968, it's much better than the current |countMe| boolean flag.)

Comment 2

[Nicholas Nethercote [:njn] (On Long Service Leave)]

Attachment: patch v3

New version, applies on top of the patch in bug 698968.

Comment 3

[Luke Wagner [:luke]]

Comment on attachment 577174 [details] [diff] [review]
patch v3

Sorry for the delay.

Comment 4

[Nicholas Nethercote [:njn] (On Long Service Leave)]

Oh crap, I was about to remove the review request because I discovered that bug 702426 broke the existing mjit-code/regexp memory reporter and so we haven't been reporting regexp memory usage since it landed.  I'll fix that in my next patch.

Comment 5

[Nicholas Nethercote [:njn] (On Long Service Leave)]

Attachment: patch v4, diff'd against v3

New patch with the following additional changes (diff'd against patch v3):

- I renamed various size-getting functions to use the new sizeOfFoo() 
  conventions, as per https://wiki.mozilla.org/Platform/Memory_Reporting.

- I added measurement of ThreadData::dtoaState, just for kicks.

- I moved explicit/js/stack under explicit/js/runtime/threads, since that's
  where it logically is in the data structures.  And I renamed it 
  stack-committed, which is more accurate.

- I added explicit/js/runtime/threads/regexp-code.  I removed the old 
  distinction between "method" and "unused" under "mjit-code".

The js/runtime reporters now look like this:

│  ├────7,311,792 B (02.76%) -- runtime
│  │    ├──4,792,320 B (01.81%) -- threads
│  │    │  ├──4,194,304 B (01.58%) -- stack-committed
│  │    │  ├────262,144 B (00.10%) -- regexp-code
│  │    │  ├────196,608 B (00.07%) -- temporary
│  │    │  └────139,264 B (00.05%) -- normal
│  │    ├──2,097,152 B (00.79%) -- atoms-table
│  │    ├────331,776 B (00.13%) -- runtime-object
│  │    └─────90,544 B (00.03%) -- contexts

Comment 6

[Luke Wagner [:luke]]

Comment on attachment 577838 [details] [diff] [review]
patch v4, diff'd against v3

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

::: js/src/assembler/jit/ExecutableAllocator.cpp
@@ +37,5 @@
>      m_allocator->releasePoolPages(this);
>  }
>  
>  void
> +ExecutableAllocator::sizeOfCode(size_t& method, size_t& regexp, size_t& unused) const

Can you pass by * instead.  Also, SM style is the * goes next to the identifier not the type.

::: js/src/jscompartment.cpp
@@ +149,5 @@
>      return true;
>  }
>  
>  void
> +JSCompartment::sizeOfCode(size_t& method, size_t& regexp, size_t& unused) const

Same comment as above.

Comment 7

[Nicholas Nethercote [:njn] (On Long Service Leave)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f1c49a3b76e7

Comment 8

[Nicholas Nethercote [:njn] (On Long Service Leave)]

Follow-up that unbreaks non-JS_THREADSAFE builds:
https://hg.mozilla.org/integration/mozilla-inbound/rev/43adf69a4a7c

Comment 9

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/f1c49a3b76e7
https://hg.mozilla.org/mozilla-central/rev/43adf69a4a7c