1501775 - Spew IC entry counters before Ion compilation

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, enhancement, P3)

Description

[Matthew Gaudet (he/him) [:mgaudet]]

It seems like a very sensible request to be able to dump the IC entry counters for a script before Ion compilation of it, in particular once we have a client for the IC entry counters in Ion.

Comment 1

[Kannan Vijayan [:djvj]]

Attachment: dump-baseline-ic-stats-on-ion-compile.patch

This changes the dump compilation to locations where we are about to ion-compile things (top-level and inlining).

It also fixes up the preetty-printing of the stubs to be a bit more readable.  Example:

```
[BaselineICStats] Dumping IC info for To-Be-Compiled script ./js/src/jit-test/tests/v8-v5/check-deltablue.js:415:45
[BaselineICStats]     functionthis - pc=0 line=415 col=0
[BaselineICStats]           ?? ->  ?? -> 
[BaselineICStats]     getprop - pc=10 line=416 col=2
[BaselineICStats]           ?? -> (fb) 2
[BaselineICStats]     getgname - pc=15 line=416 col=2
[BaselineICStats]           ?? -> (fb) 1
[BaselineICStats]     getprop - pc=20 line=416 col=2
[BaselineICStats]           ?? -> (fb) 1
[BaselineICStats]     eq - pc=25 line=416 col=2
[BaselineICStats]          987 -> (fb) 1
[BaselineICStats]     getprop - pc=36 line=416 col=2
[BaselineICStats]           ?? -> (fb) 2
[BaselineICStats]     getprop - pc=51 line=416 col=2
[BaselineICStats]           ?? -> (fb) 1
[BaselineICStats] Dumping IC info for To-Be-Compiled script ./js/src/jit-test/tests/v8-v5/check-deltablue.js:72:44
[BaselineICStats]     functionthis - pc=0 line=72 col=0
[BaselineICStats]           ?? ->  ?? -> 
[BaselineICStats]     getprop - pc=10 line=73 col=2
[BaselineICStats]           ?? -> (fb) 2
[BaselineICStats]     length - pc=15 line=73 col=2
[BaselineICStats]           ?? -> (fb) 1
```

Comment 2

[Matthew Gaudet (he/him) [:mgaudet]]

Comment on attachment 9020191 [details] [diff] [review]
dump-baseline-ic-stats-on-ion-compile.patch

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

::: js/src/jit/BaselineJIT.cpp
@@ +1232,3 @@
>          unsigned int line = PCToLineNumber(script, pc, &column);
> +        out.printf("[BaselineICStats]     %s - pc=%u line=%u col=%u\n",
> +                   CodeName[*pc], pcOffset, line, column);

The only thing I don't like about this patch is the newline instead of tab; the reason I wanted them all on one line is so that you can easily grep for a particular file/line and see the stubs there.

@@ -1255,5 @@
>          return;
>      }
>  
> -#ifdef JS_JITSPEW
> -    DumpICInfo(script);

I do wonder if maybe we need a different flag for this mode for working on baseline, but, then again, most things we're interested are those that hit ion, so perhaps Ion Compilation is the better place.

Comment 3

[Matthew Gaudet (he/him) [:mgaudet]]

I'm Ok with landing this as is though, because I want a different solution to this spew in the longer term anyhow, and am currently thinking/prototyping.

Comment 4

[Kannan Vijayan [:djvj]]

Yeah, I realize that the greppability is lost here, and we probably want a better solution overall.  Most of the time when we look through spew, it's via capturing piping to `less` (either after a file-capture or live-piping), and then grepping for particular key strings like filenames.  This should work with that.

For more structured output, I think what we want is perhaps a JSON dump that gets done to a pre-specified location.  We might want to add a generic "structured-spew-output-directory" (--spew-dir) option that gets used where particularly named files are dumped.  This would provide a decent place for the IONGRAPH spew to go as well, instead of randomly going to temp these days.

Comment 5

[Kannan Vijayan [:djvj]]

Smoke run through try to ensure no build complaints were introduced on any platforms: https://treeherder.mozilla.org/#/jobs?repo=try&revision=14c1d251e2e53053f82e9164f40229f6020dc015

Comment 6

[Matthew Gaudet (he/him) [:mgaudet]]

(In reply to Kannan Vijayan [:djvj] from comment #4)
> For more structured output, I think what we want is perhaps a JSON dump that
> gets done to a pre-specified location.  We might want to add a generic
> "structured-spew-output-directory" (--spew-dir) option that gets used where
> particularly named files are dumped.  This would provide a decent place for
> the IONGRAPH spew to go as well, instead of randomly going to temp these
> days.

It's like you're reading some of my design notes without me knowing...

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5a123d345104
Spew IC entry counters before Ion-compilation. r=mgaudet

Comment 8

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/5a123d345104