1078273 - AcornJS parser 2x slower in SpiderMonkey than v8

Status: NEW

(Core :: JavaScript Engine: JIT, defect, P5)

Description

[Till Schneidereit [:till]]

Attachment: acorn-parsing.zip

The attached archive contains a test that uses the Acorn parser[1] to parse jQuery. It's about 60% or so slower in current JS shell than in node v0.10.32:

SM:
Time to complete: 122122 microseconds.

Node:
Time to complete: 52719 microseconds.


[1] http://marijnhaverbeke.nl/acorn/

Comment 1

[Till Schneidereit [:till]]

I dug into this some, and it looks like this is largely a JIT issue.

First, I tested with --no-baseline and --no-ion, with interesting results:

js --no-baseline test.js
(SM Shell)  Time to complete: 306030 microseconds.

js --no-ion test.js
(SM Shell)  Time to complete: 117389 microseconds.

So Baseline helps a lot, but Ion actually slightly decreases performance. For kicks, let's see what --ion-eager does:

js --ion-eager test.js
(SM Shell)  Time to complete: 243551 microseconds.


IONFLAGS=abort,bailouts,bl-aborts,bl-bails doesn't show anything, which is nice.

According to IONFLAGS=bl-scripts and IONFLAGS=scripts, Baseline compiles 73 scripts, and Ion 53. That seems like a very high percentage, but Acorn consists of lots and lots of small functions, many of which are hot, I guess.

Then, there's inlining. In total, functions are inlined 819 times. The one that's inlined by far the most is `finishToken`, at line 597, with 98 times. That function has 25 callsites, so it's callees get inlined quite often, too. Inlining is vetoed 840 times, 225 of which fall on `finishToken` again.

Does inlining help at all? No, it hurts:

js --ion-inlining=off --no-ion test.js
(SM Shell)  Time to complete: 115084 microseconds.


Profiling with Instruments shows that, for an instrumented runtime of 154ms, we spend somewhat more than 100ms in off-thread compiling, across 10 threads. (Though 90ms of that are in one thread, with 12ms being the next-highest, then 7ms.)

For comparison, I looked at what v8 does with `d8 --trace-inlining`: it inlines functions 38, 28 of which are `finishToken`. They veto finishToken 6 times, always with "cumulative AST node limit reached" as the reason. Some other functions are vetoed with "inline depth limit reached", so their heuristics do very different things here.

ni?h4writer because AFAIK he looked into inlining heuristics last.

Comment 2

[Hannes Verschore [:h4writer]]

Not clearing NI. Since I need to dig further into this.

1) It seems the benchmark doesn't take long enough.
We are still compiling. I.e. we are mostly running in baseline, with interruptions of IonBuilder to prepare generating IM code on the background thread. So we are taking the hit of starting to compile, but the compilations aren't quite done yet. So we don't have the improved performance yet. This is offcourse exaggerated by enabling inlining, because IonBuilder needs to run longer.
=> So that would explain the drop in performance w/wo inlining.

Note there is constructional problem here. Only during compilation we can decide to allow/disallow inlining. So we are quite aggressive on inlining (even if a function isn't hot), since else it could be a missed oppertunity. With bug 911738 I hoped to alter that burden and to allow inlining after compilation. This has taken some time, but should be ready now.
This would allow us to compile more aggressive (without inlining) and allow recompilation with inlining when an function gets hot. This will needs some careful measurements so we don't degrade performance. But could improve our startup time. (i.e.. improve performance for scripts that are just hot and we decide to stop running)

To have more meaningful data I ran this benchmark multiple times (so the issue in IM is visible instead of startup performance):

2) Run 100x:
> Spidermonkey: 3089ms (with inlining)
> SpiderMonkey: 3041ms (without inlining)
> Chrome:       1447ms

Two issues we should address and look into:
- Why is there no improvement with inlining! This is quite strange. Inlining should bring gains
- Why do we take twice the time as chrome.

Comment 3

[Hannes Verschore [:h4writer]]

Attachment: Add eager optimization level

This is to address (1)
This adds an optimization level at warmupcount 100, which creates an IonScript which doesn't inline anything. The compilation of this is finished quickly and so we don't have to wait till full IonScript (with inlined functions) before starting to run in IM.

Running once:
IM Before: 110ms
IM After:   83ms
IM Extra:   76ms (delay full compilation till warmupcount 10.000 *)
d8          62ms

* Like said this will need some careful measurements, since for benchmarks that take long enough, eager-compilation is the best strategy.

Comment 4

[Till Schneidereit [:till]]

Hannes, this seems to show that bug 1080776 might be quite useful for real-world code.

Comment 5

[Mayank Bansal]

We are still quite a bit slower here.

Nightly: https://share.firefox.dev/3ADfYeA
Acorn (dev) Acorn 8.12.1
angular.js 18.88 ops/sec 19.28 ops/sec
backbone.js 243.92 ops/sec 255.90 ops/sec
ember.js 8.14 ops/sec 7.95 ops/sec
jquery.js 41.01 ops/sec 41.51 ops/sec
react-dom.js 22.54 ops/sec 22.08 ops/sec
react.js 148.35 ops/sec 149.09 ops/sec

Chrome:
Acorn (dev) Acorn 8.12.1
angular.js 40.48 ops/sec 38.16 ops/sec
backbone.js 507.98 ops/sec 523.74 ops/sec
ember.js 15.10 ops/sec 15.34 ops/sec
jquery.js 97.86 ops/sec 84.23 ops/sec
react-dom.js 50.92 ops/sec 47.35 ops/sec
react.js 285.03 ops/sec 234.52 ops/sec

But not sure if this is something that should be focussed on.

Comment 6

[Mayank Bansal]

Attachment: Screenshot on 2024-08-23 at 11:03:30.png

Comment 7

[Mayank Bansal]

Nightly: https://share.firefox.dev/41Ud7cd (70 seconds)
Chrome: https://share.firefox.dev/41IOe1L (45s)

65% tenuring rate in the profile