Refine JS memory reporters

RESOLVED FIXED in mozilla10

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: njn, Assigned: njn)

Tracking

Trunk
mozilla10
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [MemShrink])

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

6 years ago
Created attachment 569287 [details] [diff] [review]
patch

This draft patch refines the JS memory reporters.

- "objects" is sub-divided into "function" and "non-function".

- "shapes" is sub-divided into "tree" and "dict".

- "property-tables" is sub-divided into "shapes-extra/tree-tables" and "shapes-extra/dict-tables".

- "shape-kids" is changed to "shapes-extra/tree-shape-kids"

- "arena-*" are changed to "arena/*"

I still need to fix the inDictionary() hack.


Here's an example of the new output:

│   ├──33,010,352 B (25.10%) -- compartment(https://mail.google.com/mail/u/0/?shva=1#inbox)
│   │  ├──18,681,856 B (14.21%) -- gc-heap
│   │  │  ├───6,380,752 B (04.85%) -- objects
│   │  │  │   ├──3,291,984 B (02.50%) -- function
│   │  │  │   └──3,088,768 B (02.35%) -- non-function
│   │  │  ├───4,064,032 B (03.09%) -- scripts
│   │  │  ├───3,725,120 B (02.83%) -- shapes
│   │  │  │   ├──2,335,168 B (01.78%) -- tree
│   │  │  │   └──1,389,952 B (01.06%) -- dict
│   │  │  ├───3,672,048 B (02.79%) -- arena
│   │  │  │   ├──3,314,072 B (02.52%) -- unused
│   │  │  │   ├────212,024 B (00.16%) -- padding
│   │  │  │   └────145,952 B (00.11%) -- headers
│   │  │  ├─────568,768 B (00.43%) -- type-objects
│   │  │  └─────271,136 B (00.21%) -- strings
│   │  ├───4,534,640 B (03.45%) -- script-data
│   │  ├───3,272,608 B (02.49%) -- type-inference
│   │  │   ├──1,986,064 B (01.51%) -- script-main
│   │  │   ├──1,121,808 B (00.85%) -- object-main
│   │  │   └────164,736 B (00.13%) -- tables
│   │  ├───2,359,296 B (01.79%) -- mjit-code
│   │  │   ├──2,262,504 B (01.72%) -- method
│   │  │   ├─────96,040 B (00.07%) -- regexp
│   │  │   └────────752 B (00.00%) -- unused
│   │  ├───2,155,904 B (01.64%) -- shapes-extra
│   │  │   ├──1,440,288 B (01.10%) -- tree-tables
│   │  │   ├────369,376 B (00.28%) -- dict-tables
│   │  │   └────346,240 B (00.26%) -- tree-shape-kids
│   │  ├───1,324,024 B (01.01%) -- object-slots
│   │  ├─────368,328 B (00.28%) -- analysis-temporary
│   │  ├─────176,320 B (00.13%) -- string-chars
│   │  └─────137,376 B (00.10%) -- object-empty-shapes
(Assignee)

Comment 1

6 years ago
Created attachment 569901 [details] [diff] [review]
patch

I don't much like making inDictionary public, but I don't see a better way to do it.
Attachment #569287 - Attachment is obsolete: true
Attachment #569901 - Flags: review?(bhackett1024)
(Assignee)

Comment 2

6 years ago
Created attachment 569902 [details] [diff] [review]
patch 2

This patch adds numbers to the "other measurements" list giving total memory
used for JS objects, scripts, shapes, strings, and the method JIT.  This
seems interesting.  I could also do type inference, but I wasn't sure
whether to include "analysis-temporary" and "object-empty-shapes" on top of
"type-inference/*"
Attachment #569902 - Flags: review?(bhackett1024)
Comment on attachment 569901 [details] [diff] [review]
patch

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

::: js/src/jsscope.h
@@ +536,4 @@
>          PUBLIC_FLAGS    = HAS_SHORTID | METHOD
>      };
>  
> +    JS_FRIEND_API(bool) inDictionary() const   { return (flags & IN_DICTIONARY) != 0; }

Is the JS_FRIEND_API needed here?  inDictionary() is an inline function, and other inlines used by XPCJSRuntime like obj->isFunction() don't have this.

FWIW jsscope.h and jsobj.h are no longer installed headers and shouldn't be used outside js/src, but js/xpconnect/src has js/src in its LOCAL_INCLUDES.  This will get fixed pretty soon by bug 692277, but that doesn't need to affect this patch.

::: js/xpconnect/src/XPCJSRuntime.cpp
@@ +1688,2 @@
>                         "Memory on the compartment's garbage-collected JavaScript heap that holds "
> +                       "shapes that are in a property tree.",

Bit of a nit, but it would be nice if the property tree and dictionaries had a short explanation, so someone who isn't deeply familiar with the engine can follow along.

"The property tree encodes the property layout of objects in a way that is shareable among many objects."

"Dictionaries encode the property layout of particular objects."
Attachment #569901 - Flags: review?(bhackett1024) → review+
Comment on attachment 569902 [details] [diff] [review]
patch 2

Nice, I've had to write scrapers to get this data before.  For TI, I think that the analysis-temporary should be a separate category from type-inference/*, and object-empty-shapes should go in the shapes category.
Attachment #569902 - Flags: review?(bhackett1024) → review+
(In reply to Brian Hackett from comment #3)
> Is the JS_FRIEND_API needed here?  inDictionary() is an inline function, and
> other inlines used by XPCJSRuntime like obj->isFunction() don't have this.

From my previous poking around, JS_FRIEND_API specifies linker behavior on Windows (and does nothing elsewhere), so I'd think it wouldn't be needed for inline functions, at least in terms of getting things to work (no idea what the style calls for).
(Assignee)

Comment 6

6 years ago
Created attachment 570150 [details] [diff] [review]
patch 2, v2

Relative to patch 2, this patch:

- Renames all the "total" counts so the total is at the front (e.g. 
  "js-objects-total" becomes "js-total-objects", which ensures
  they're listed together.
  
- Renames "object-empty-shapes" as "shapes-extra/empty-shape-arrays", and 
  includes it in "js-total-shapes".

- Adds "js-total-type-inference" and "js-total-analysis-temporary".
Attachment #569902 - Attachment is obsolete: true
Attachment #570150 - Flags: review?(bhackett1024)
Attachment #570150 - Flags: review?(bhackett1024) → review+
(Assignee)

Updated

6 years ago
Whiteboard: [MemShrink]
(Assignee)

Comment 7

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/343cb6ea05bd
https://hg.mozilla.org/integration/mozilla-inbound/rev/6ec5b28142d1
https://hg.mozilla.org/mozilla-central/rev/343cb6ea05bd
https://hg.mozilla.org/mozilla-central/rev/6ec5b28142d1
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
OS: Linux → All
Hardware: x86_64 → All
Resolution: --- → FIXED
Target Milestone: --- → mozilla10
Version: unspecified → Trunk
You need to log in before you can comment on or make changes to this bug.