More detail in JS memory reports

RESOLVED FIXED in mozilla19

Status

()

defect
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: njn, Assigned: njn)

Tracking

unspecified
mozilla19
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [js:t])

Attachments

(5 attachments)

Assignee

Description

7 years ago
I have some patches to add yet more detail to JS memory reports.
Assignee

Comment 1

7 years ago
This patch renames the "cross-compartment-wrappers" memory report as
"cross-compartment-wrappers-table", which is a better name because it's just
the table, but not the CCW objects themselves.
Attachment #675636 - Flags: review?(luke)
Assignee

Comment 2

7 years ago
This patch distinguishes dense array objects, slow array objects, and
cross-compartment wrapper objects from "ordinary" objects.
Attachment #675638 - Flags: review?(luke)

Updated

7 years ago
Attachment #675636 - Flags: review?(luke) → review+
Assignee

Comment 3

7 years ago
JSString::isShort() succeeds for strings that are not short but happen to be
in a FINALIZE_SHORT_STRING arena.  This patch fixes this misleading behaviour.
Attachment #675641 - Flags: review?(luke)

Updated

7 years ago
Attachment #675638 - Flags: review?(luke) → review+
Assignee

Comment 4

7 years ago
This patch distinguishes short strings from normal strings.
Attachment #675645 - Flags: review?(luke)
Comment on attachment 675641 [details] [diff] [review]
(part 3) - Fix misleading behaviour of JSString::isShort().

Thanks!
Attachment #675641 - Flags: review?(luke) → review+
Assignee

Comment 6

7 years ago
This patch distinguishes tree shapes that don't have the global object as 
their parent.
Attachment #675646 - Flags: review?(luke)
Comment on attachment 675645 [details] [diff] [review]
(part 4) - Add more detail to string memory reports.

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

::: js/src/vm/String.h
@@ +415,4 @@
>  
>      static inline js::ThingRootKind rootKind() { return js::THING_ROOT_STRING; }
>  
> +    bool isShort() const;

Could you move this declaration next to the other isX() queries (below asInline, before asStable).

::: js/xpconnect/src/XPCJSRuntime.cpp
@@ +1509,5 @@
> +    CREPORT_GC_BYTES(cJSPathPrefix + NS_LITERAL_CSTRING("gc-heap/strings/short"),
> +                     cStats.gcHeapStringsShort,
> +                     "Memory on the garbage-collected JavaScript "
> +                     "heap that holds short strings, in which the string "
> +                     "characters are stored in the string header.");

How about:

"Memory on the garbage-collected JavaScript heap that holds double-sized string headers which store the characters inline."
Attachment #675645 - Flags: review?(luke) → review+
Hmm, "double-sized" might be confusing (with the C++ type 'double').  How about "over-sized"?
Comment on attachment 675646 [details] [diff] [review]
(part 5) - Add more detail to shape memory reports.

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

::: js/xpconnect/src/XPCJSRuntime.cpp
@@ +1522,5 @@
> +    CREPORT_GC_BYTES(cJSPathPrefix + NS_LITERAL_CSTRING("gc-heap/shapes/tree/canonical"),
> +                     cStats.gcHeapShapesTreeCanonical,
> +                     "Memory on the garbage-collected JavaScript heap that "
> +                     "holds shapes that are in a property tree, and whose "
> +                     "parent is the global object.");

How about "Memory on the garbage-collected JavaScript heap that holds shapes that are in the property tree and represent an object whose parent is the global object" (and the same below, mutatis mutandis)?
Attachment #675646 - Flags: review?(luke) → review+
You need to log in before you can comment on or make changes to this bug.