789398 - Rework type inference memory reporters

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Nicholas Nethercote [inactive]]

Bug 781767 comment 4 suggests that the type inference memory reporters could do with some reworking.

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: Bug 789398 - Rework the type inference memory reporters.

I'm not sure what my timing is like with this patch -- has bhackett left for
his long vacation yet?

This patch:

- Makes the type-inference memory reporters more fine-grained, going from
  four distinct measurements to nine.  In TypeInferenceSizes:

    scripts --> typeScripts + typeResults

    temporary --> analysisPool + typePool + pendingArrays

    tables --> allocationSiteTables + arrayTypeTables + objectTypeTables

    objects --> typeObjects

  The names I used were heavily based on the relevant types.  I haven't
  bothered with descriptions for each of the measurements yet, I figured I'd
  wait to see what the initial reaction is.

- It no longer subtracts some of the bytes measured in |temporary| and adds
  them to |scripts| and |objects|.  Computing sizes analytically is
  error-prone, and in this case leads to bug 775382.

- I've moved the temporary reporters into the "type-inference" sub-tree for
  the simple reason that I don't understand why they weren't there in the
  first place.  I'm happy to hear arguments against this move.

Here's an example with a few sites open:

│  ├───9,337,696 B (09.53%) -- type-inference
│  │   ├──5,767,168 B (05.89%) ── type-pool
│  │   ├──2,883,584 B (02.94%) ── analysis-pool
│  │   ├────311,888 B (00.32%) ── type-scripts
│  │   ├────293,648 B (00.30%) ── allocation-site-tables
│  │   ├─────60,752 B (00.06%) ── object-type-tables
│  │   ├─────12,464 B (00.01%) ── array-type-tables
│  │   └──────8,192 B (00.01%) ── pending-arrays

I guess "type-results" and "type-objects" were small enough that they got
folded into "other-sundries" (not shown).

Comment 3

[Brian Hackett [Laid off!]]

Comment on attachment 659138 [details] [diff] [review]
Bug 789398 - Rework the type inference memory reporters.

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

Sorry for the super late review!  This came in just after I left on my trip, and I forgot to take care of it during the work week.

::: js/src/jsinfer.cpp
@@ +6231,5 @@
>      /*
>       * Note: not all data in the pool is temporary, and some will survive GCs
>       * by being copied to the replacement pool. This memory will be counted
>       * elsewhere and deducted from the amount of temporary data.
>       */

This comment is obsolete now.

::: js/xpconnect/src/XPCJSRuntime.cpp
@@ +1547,5 @@
>                    "Memory used by cross-compartment wrappers.");
>  
> +    CREPORT_BYTES(cJSPathPrefix + NS_LITERAL_CSTRING("type-inference/type-scripts"),
> +                  cStats.typeInferenceSizes.typeScripts,
> +                  "XXX.");

"Memory for type sets associated with scripts"

@@ +1552,4 @@
>  
> +    CREPORT_BYTES(cJSPathPrefix + NS_LITERAL_CSTRING("type-inference/type-results"),
> +                  cStats.typeInferenceSizes.typeResults,
> +                  "XXX.");

"Memory for dynamic type results produced by scripts"

@@ +1556,4 @@
>  
> +    CREPORT_BYTES(cJSPathPrefix + NS_LITERAL_CSTRING("type-inference/analysis-pool"),
> +                  cStats.typeInferenceSizes.analysisPool,
> +                  "XXX.");

"Memory holding transient analysis information used during type inference and compilation"

@@ +1560,4 @@
>  
> +    CREPORT_BYTES(cJSPathPrefix + NS_LITERAL_CSTRING("type-inference/type-pool"),
> +                  cStats.typeInferenceSizes.typePool,
> +                  "XXX.");

"Memory holding contents of type sets and related data"

@@ +1563,5 @@
> +                  "XXX.");
> +
> +    CREPORT_BYTES(cJSPathPrefix + NS_LITERAL_CSTRING("type-inference/pending-arrays"),
> +                  cStats.typeInferenceSizes.pendingArrays,
> +                  "XXX.");

"Memory used for solving constraints during type inference"

@@ +1567,5 @@
> +                  "XXX.");
> +
> +    CREPORT_BYTES(cJSPathPrefix + NS_LITERAL_CSTRING("type-inference/allocation-site-tables"),
> +                  cStats.typeInferenceSizes.allocationSiteTables,
> +                  "XXX.");

"Memory indexing type objects associated with allocation sites"

@@ +1571,5 @@
> +                  "XXX.");
> +
> +    CREPORT_BYTES(cJSPathPrefix + NS_LITERAL_CSTRING("type-inference/array-type-tables"),
> +                  cStats.typeInferenceSizes.arrayTypeTables,
> +                  "XXX.");

"Memory indexing type objects associated with array literals"

@@ +1575,5 @@
> +                  "XXX.");
> +
> +    CREPORT_BYTES(cJSPathPrefix + NS_LITERAL_CSTRING("type-inference/object-type-tables"),
> +                  cStats.typeInferenceSizes.objectTypeTables,
> +                  "XXX.");

"Memory indexing type objects associated with object literals"

@@ +1579,5 @@
> +                  "XXX.");
> +
> +    CREPORT_BYTES(cJSPathPrefix + NS_LITERAL_CSTRING("type-inference/type-objects"),
> +                  cStats.typeInferenceSizes.typeObjects,
> +                  "XXX.");

"Miscellaneous additional information associated with type objects"

Comment 4

[Nicholas Nethercote [inactive]]

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

Comment 5

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/e10975ff4a07