Closed Bug 1501775 Opened Last year Closed Last year

Spew IC entry counters before Ion compilation

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
mozilla65
Tracking Status
firefox65 --- fixed

People

(Reporter: mgaudet, Assigned: djvj)

References

Details

Attachments

(1 file)

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.
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
```
Assignee: nobody → kvijayan
Attachment #9020191 - Flags: review?(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.
Attachment #9020191 - Flags: review?(mgaudet) → review+
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.
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.
Smoke run through try to ensure no build complaints were introduced on any platforms: https://treeherder.mozilla.org/#/jobs?repo=try&revision=14c1d251e2e53053f82e9164f40229f6020dc015
(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...
Pushed by kvijayan@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5a123d345104
Spew IC entry counters before Ion-compilation. r=mgaudet
https://hg.mozilla.org/mozilla-central/rev/5a123d345104
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
You need to log in before you can comment on or make changes to this bug.