Closed Bug 865406 Opened 7 years ago Closed 7 years ago

Hundreds of MUnbox bailouts on v8-crypto

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla23

People

(Reporter: jandem, Assigned: jandem)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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.
I noticed a similar issue. I was thinking about another solution: bug 857360
(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|).
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 ...
Attached patch PatchSplinter Review
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.
Attachment #741757 - Flags: review?(hv1989)
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.
Attachment #741757 - Flags: review?(hv1989) → review+
Ah and what's the performance increase estimate?
https://hg.mozilla.org/mozilla-central/rev/17b205981e76
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
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?
Flags: needinfo?(mrosenberg)
Depends on: 878019
Flags: needinfo?(mrosenberg)
You need to log in before you can comment on or make changes to this bug.