865406 - Hundreds of MUnbox bailouts on v8-crypto

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jan de Mooij [:jandem]]

Looks like we are hoisting an MUnbox(String) from inside an if-statement, then keep bailing out hundreds of times because the value is an object sometimes (but we never execute the "if" in that case so we don't invalidate anything).

We need some mechanism/heuristic to disable LICM when this happens too frequently. Eventually we could do this only for instructions that can bailout.

Comment 1

[Hannes Verschore [:h4writer]]

I noticed a similar issue. I was thinking about another solution: bug 857360

Comment 2

[Jan de Mooij [:jandem]]

(In reply to Hannes Verschore [:h4writer] from comment #1)
> I noticed a similar issue. I was thinking about another solution: bug 857360

The problem I think is that if we stop hoisting instructions inside if-statements we will regress other benchmarks, and even without if-statements there can be similar issues if the loop never executes (because the condition is always |false|).

Comment 3

[Hannes Verschore [:h4writer]]

The intention was not to disable hoisting instructions inside if-statements. 
But only if we can detect the condition restricts the values flowing in.

E.g.
val === Undefined, if the typeset of value is bigger than Undefined
val === 0, if the typeset of value includes more than integers

There is still an unknown if we want to be pro-active or not. E.g. also disabling if we are not sure if it restricts values flowing. This need to get checked with the benchmarks and actual scripts, what would be better.

Actually now that I think about it, we can implement the not-eagerly version and always run that and with some mechanism/heuristic (like you want to implement here) when we see we are still hitting this problem, enable eagerly disabling hoist instruction for unknown if-statements ...

Comment 4

[Jan de Mooij [:jandem]]

Attachment: Patch

Disable LICM if a script bails out at least 10 times without any invalidation.

We could experiment with what you proposed in comment 3, but this happens on very few benchmarks so I think this is the simplest fix for now.

Comment 5

[Hannes Verschore [:h4writer]]

Comment on attachment 741757 [details] [diff] [review]
Patch

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

Looks fine. Could you check if we have scripts that bailout frequently and we disabled the LICM optimization, but we still bail often? So where disabling LICM wasn't the cure for this bailouts. It's just to have an overview.

Comment 6

[Hannes Verschore [:h4writer]]

Ah and what's the performance increase estimate?

Comment 7

[Jan de Mooij [:jandem]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/17b205981e76

Try: https://tbpl.mozilla.org/?tree=Try&rev=bb15edd40f16

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/17b205981e76

Comment 9

[Hannes Verschore [:h4writer]]

This caused a 42.6% regression on ARM octane-raytrace.

(In reply to Jan de Mooij [:jandem] from comment #4)
> Disable LICM if a script bails out at least 10 times without any
> invalidation.

Could it be that we are bailing more in ARM for some reason? And for which reasons?