846733 - BaselineCompiler: Add memory reporters

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jan de Mooij [:jandem]]

Attachment: Patch

This patch adds two memory reporters: baseline-data is the size of all BaselineScript's and baseline-stubs is the size of the IC stubs.

Like JM and Ion, JIT code is measured separately. Since we use IonCode, baseline JIT code shows up as "ion-code" for now.

Comment 1

[Kannan Vijayan [:djvj]]

Comment on attachment 719920 [details] [diff] [review]
Patch

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

Looks good.

Comment 2

[Nicholas Nethercote [inactive]]

Comment on attachment 719920 [details] [diff] [review]
Patch

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

Please r? me on memory reporter patches in the future -- thanks!

::: js/src/ion/BaselineJIT.h
@@ +292,5 @@
>  void
>  FinishDiscardBaselineScript(FreeOp *fop, UnrootedScript script);
>  
> +void
> +BaselineMemoryUsed(JSScript *script, JSMallocSizeOfFun mallocSizeOf, size_t *data, size_t *stubs);

The convention for memory reporters is for all measuring functions to include "[Ss]izeOf" in the name.  So this should be "SizeOfBaselineScript" or something like that.

::: js/xpconnect/src/XPCJSRuntime.cpp
@@ +1727,5 @@
>  
> +    CREPORT_BYTES(cJSPathPrefix + NS_LITERAL_CSTRING("baseline-data"),
> +                  cStats.baselineData,
> +                  "Memory used by the Baseline JIT for compilation data: "
> +                  "BaselineScripts.");

"data" is pretty meaningless.  How about "baseline-scripts"?

Comment 3

[Jan de Mooij [:jandem]]

(In reply to Nicholas Nethercote [:njn] from comment #2)
> Please r? me on memory reporter patches in the future -- thanks!

Sorry, I will do that next time. Thanks for the comments.

> The convention for memory reporters is for all measuring functions to
> include "[Ss]izeOf" in the name.  So this should be "SizeOfBaselineScript"
> or something like that.

OK, I used "BaselineMemoryUsed" to match ion::MemoryUsed, but I like SizeOfBaselineScript. I will change it.

> "data" is pretty meaningless.  How about "baseline-scripts"?

I used "baseline-data" to match the "jaeger-data" and "ion-data" reporters... I'm fine with "baseline-scripts" if you think it's okay to deviate from that?

Comment 4

[Nicholas Nethercote [inactive]]

> OK, I used "BaselineMemoryUsed" to match ion::MemoryUsed, but I like
> SizeOfBaselineScript. I will change it.

r=me if you want to change ion::MemoryUsed to ion::SizeOfFoo, where "Foo" is "Data" or something similar.

> > "data" is pretty meaningless.  How about "baseline-scripts"?
> 
> I used "baseline-data" to match the "jaeger-data" and "ion-data"
> reporters...

Oh, fair enough.  Keep "baseline-data" then.

Thanks!

Comment 5

[Jan de Mooij [:jandem]]

https://hg.mozilla.org/projects/ionmonkey/rev/078d5100b491

(I changed it to SizeOfIonData and SizeOfBaselineData.)