BaselineCompiler: Separate ion/baseline memory reporters for JIT code

RESOLVED FIXED in mozilla22

Status

()

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: jandem, Assigned: jandem)

Tracking

unspecified
mozilla22
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
Right now we use IonCode so baseline code is reported under ion-code. njn suggested using different reporters for baseline and Ion code, and I agree that would be useful.
(Assignee)

Comment 1

6 years ago
Posted patch PatchSplinter Review
Adds two new memory reporters: code/other and code/baseline (and updates the other reporters so that they are in a "code" subtree). code/other is used for trampolines and VM wrappers. These will be used by baseline too so it's wrong to report them under either code/ion or code/baseline.
Attachment #723905 - Flags: review?(n.nethercote)
Attachment #723905 - Flags: review?(dvander)
Comment on attachment 723905 [details] [diff] [review]
Patch

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

::: js/src/assembler/jit/ExecutableAllocator.cpp
@@ +49,5 @@
> +    *ion      = 0;
> +    *baseline = 0;
> +    *regexp   = 0;
> +    *other    = 0;
> +    *unused   = 0;

Six values is enough that this would benefit from getting its own struct, like ObjectsExtraSizes and TypeInferenceSizes.  Feel free to do that if you want, but I won't insist.

::: js/xpconnect/src/XPCJSRuntime.cpp
@@ +1881,5 @@
>  
> +    RREPORT_BYTES(rtPath + NS_LITERAL_CSTRING("runtime/code/baseline"),
> +                  nsIMemoryReporter::KIND_NONHEAP, rtStats.runtime.baselineCode,
> +                  "Memory used by the Baseline JIT to hold the runtime's "
> +                  "generated code.");

You can remove "the runtime's " here and in the other cases.
Attachment #723905 - Flags: review?(n.nethercote) → review+
Comment on attachment 723905 [details] [diff] [review]
Patch

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

Why OTHER_CODE instead of ION_CODE?
Attachment #723905 - Flags: review?(n.nethercote)
Attachment #723905 - Flags: review?(dvander)
Attachment #723905 - Flags: review+
Comment on attachment 723905 [details] [diff] [review]
Patch

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

My r+ was overwritten, apparently.
Attachment #723905 - Flags: review?(n.nethercote) → review+
(Assignee)

Comment 5

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/40b366dc7fad

(In reply to David Anderson [:dvander] from comment #3)
> Why OTHER_CODE instead of ION_CODE?

VM wrappers etc are used by both Ion and baseline, so I think it's clearer to report them under OTHER instead of ION.
ION_OR_BASELINE_CODE, maybe?
https://hg.mozilla.org/mozilla-central/rev/40b366dc7fad
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
You need to log in before you can comment on or make changes to this bug.