848849 - BaselineCompiler: Bad behaviour with parallel ion compile

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Hannes Verschore [:h4writer]]

I noticed this on v8-regexp, but I assume there are other places where this happens.

V8-regexp:120, runBlock0() runs the first time in the baseline compiler. And it has enough loops to increase the usecount to compile in IonMonkey. But it will never been able to complete compiling, as it gets cancelled and rescheduled the whole time. 

So when we reach a high enough usecount the following happens
1) Execute Opcode in Baseline
2) Baseline decides it's time to compile in IM
3) Runs IonBuilder
4) Starts compiling offthread
5) Goto Next Opcode
6) See a new type
7) Cancel all offthread compilations
8) GOTO 1

So we are restarting the offthread compilation a lot! This doesn't happen on every opcode, but it happens a lot, because the types still change a lot. The result is that we are constantly running IonBuilder and that we never go to the IM code (the first time). For runBlock0 there are 23 recompilations just for the first execution of the function.

2 problems here:
1) UseCount is a bad way to know if a function types has been stabilized. In this case it hasn't yet. But we already knew that ...
2) Baseline doesn't reset the usecount or doesn't somehow add a breathing time AFTER a new type is added to see what this new type will do with the rest of the code. It immediately reschedules the IM recompilation.

Comment 1

[Jan de Mooij [:jandem]]

Attachment: Patch

I just ran into this when investigating bug 850595. This patch resets the use count when type information changes. Seems to win 3-4% on SS, mostly on the tests that regress the most in the browser:

    fannkuch:          1.31x as fast       7.8ms +/- 1.3%     6.0ms +/-

    base64:            1.24x as fast       5.6ms +/- 1.9%     4.6ms +/-

Comment 2

[Jan de Mooij [:jandem]]

Pushed, but backed out again: tests are failing because resetting the use count during Ion-compilation makes IonBuilder insert a recompile check and bailout-to-baseline asserts... Updated patch tomorrow.

https://hg.mozilla.org/projects/ionmonkey/rev/f0f398255b65
https://hg.mozilla.org/projects/ionmonkey/rev/a34329707345

Comment 3

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(In reply to Jan de Mooij [:jandem] from comment #2)
> Pushed, but backed out again: tests are failing because resetting the use
> count during Ion-compilation makes IonBuilder insert a recompile check and
> bailout-to-baseline asserts... Updated patch tomorrow.
> 
> https://hg.mozilla.org/projects/ionmonkey/rev/f0f398255b65
> https://hg.mozilla.org/projects/ionmonkey/rev/a34329707345

There is a failing testcase in bug 850955.

Comment 4

[Jan de Mooij [:jandem]]

Attachment: Patch v2

For now don't insert a recompile check if baseline is enabled. I can file a bug to remove insertRecompileCheck, but I'd rather do that after merging to m-c.

Comment 5

[Brian Hackett [Laid off!]]

Comment on attachment 724866 [details] [diff] [review]
Patch v2

It seems like you could remove insertRecompileCheck on trunk then merge it to baseline, though that would take longer.  Your call.

Comment 6

[Jan de Mooij [:jandem]]

https://hg.mozilla.org/projects/ionmonkey/rev/725c137c988a

(In reply to Brian Hackett (:bhackett) from comment #5)
> Comment on attachment 724866 [details] [diff] [review]
> It seems like you could remove insertRecompileCheck on trunk then merge it
> to baseline, though that would take longer.  Your call.

Fair enough, filed bug 851053.