807464 - IonMonkey: Bump the checked size or find a better heuristic.

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Nicolas B. Pierron [:nbp]]

This prevent the compilation of 2 hot functions in PdfJS.  The current limit is set at 1500.

[Abort] Script too large (1782 bytes)
[Abort] Prevent compilation of pdfjs.js:16934
[Abort] Disabling Ion compilation of script pdfjs.js:16934

[Abort] Script too large (1678 bytes)
[Abort] Prevent compilation of pdfjs.js:27687
[Abort] Disabling Ion compilation of script pdfjs.js:27687

At the same time it would be better to move the constants to IonOptions in Ion.h.

Comment 1

[Sean Stangl [:sstangl]]

Yes, investigating.

Comment 2

[Nicolas B. Pierron [:nbp]]

Bumping the limit to 2000 is slowing down the script and the spew of the 2 functions indicates that

(In reply to Nicolas B. Pierron [:pierron] [:nbp] from comment #0)
> [Abort] Script too large (1782 bytes)
> [Abort] Prevent compilation of pdfjs.js:16934
> [Abort] Disabling Ion compilation of script pdfjs.js:16934

This function cannot compile because Bug 807443.

> [Abort] Script too large (1678 bytes)
> [Abort] Prevent compilation of pdfjs.js:27687
> [Abort] Disabling Ion compilation of script pdfjs.js:27687

And this function hit a type barrier after a while before being recompiled at the end of the program.

Comment 3

[Sean Stangl [:sstangl]]

Attachment: Graph of Script Length VS Compilation Time

Attached is a graph of script length versus total compilation time. Data is sampled from SS, V8, Kraken, and the Emscripten bbread, box2d, and bullet benchmarks from Alon's misc-js-benchmarks repository. Note that a number of Emscripten functions trigger bad behavior in regalloc and phi-elimination -- all the outliers are from Emscripten. I have not yet diagnosed them further.

The green line is an attempt at a best fit:

> f(x) = a * (x**b), where
> a = 0.0987429
> b = 1.38869

Based on this fit, we have the following samples of expected compilation times:

> length  250:  0.2ms
> length  500:  0.6ms
> length 1000:  1.4ms
> length 1500:  2.5ms
> length 2000:  3.8ms
> length 4000:  9.9ms
> length 6000: 17.4ms

Before we draw conclusions from this data, let's look first at the effects from removing the limit on permissible script size, assuming the same definition of function hotness:

- SS is completely unaffected; we already compile everything that's hot.

- V8 gains compilation of regexp.js:120 and regexp.js:240, both with size < 2000; performance is unaffected.

- Kraken gains compilation of audio-beat-detection-data.js:2147, with size 4047; it takes an average of 10.4 ms to compile, we compile it 4x, and performance regresses on top of that, from 304ms -> 380ms. 42% of that compilation time is in regalloc.

- Emscripten is therefore responsible for nearly every function with length > 2000 -- but, the vast majority of Emscripten functions are < 2000 in length, with a cutoff quite close to 2000. The thick red point cloud in the graph that ends at 2000 is also present if we restrict data to solely emscripten benchmarks.

Increasing the script limit from 1500 to 2000 improves BananaBread performance from an average frame length of 13.5ms to an average frame length of 12.4ms. Startup time is also improved by 600ms.

So, based on the observations that:

1) From the graph, most hot functions are <= 2000 in length
2) Functions > 2000 in length tend to have unpredictable compile times
3) Increasing the limit to 2000 doesn't affect SS, V8, or Kraken
4) Increasing the limit to 2000 helps BananaBread

I would recommend that we just increase the limit to 2000.

Comment 4

[Sean Stangl [:sstangl]]

Attachment: Patch used to dump compilation timing.

Comment 5

[Sean Stangl [:sstangl]]

Attachment: Python script to analyze timer output

The bottom of the file contains a number of possible analyses that I ran at various points, commented out.

Comment 6

[Sean Stangl [:sstangl]]

Attachment: Increase script size limit to 2000.

Comment 7

[Nicolas B. Pierron [:nbp]]

Comment on attachment 679411 [details] [diff] [review]
Increase script size limit to 2000.

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

Can you move these constants to jsOptions ?

Comment 8

[Sean Stangl [:sstangl]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/862fe97bd3a4

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/862fe97bd3a4