1760605 - Fix some issues with JSOp::FunCall/FunApply ops

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P2)

Description

[Jan de Mooij [:jandem]]

We currently only optimize these ops with ICs if we're calling the expected call/apply native function. This results in perf cliffs if we're calling a different function and also makes us more likely to run out of stack space (bug 1759412).

After Warp and the arguments analysis overhaul, it's now possible to remove JSOp::FunApply and to optimize more at JSOp::FunCall ops.

Comment 1

[Jan de Mooij [:jandem]]

Attachment: Bug 1760605 part 1 - Tidy up InlineFrameIterator::findNextFrame. r?iain!

• Turn JSOp::FunApply check into an assertion because we don't inline there anymore.
• Only call GET_ARGC for IsInvokeOp ops.
• Turn final else-if statement into a release assertion.

We now always set numActualArgs_ in this loop, so we can delete the assertion
(when we had inlining for apply-arguments, we reused the value from the caller frame).

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Bug 1760605 part 2 - Don't trial inline FunApply{ArgsObj,Array} calls. r?iain!

The goal is to replace the JSOp-based check with something else so that we can remove
the JSOp.

Depends on D141614

Comment 3

[Jan de Mooij [:jandem]]

Attachment: Bug 1760605 part 3 - Optimize fun.apply for normal Call ops. r?iain!

Also support the other call optimizations for JSOp::FunApply ops.

Depends on D141615

Comment 4

[Jan de Mooij [:jandem]]

Attachment: Bug 1760605 part 4 - Remove JSOp::FunApply. r?iain!

Depends on D141616

Comment 5

[Jan de Mooij [:jandem]]

Attachment: Bug 1760605 part 5 - Ensure matching JSOp and CallFlags for FunCall. r?iain!

If either the JSOp or CallFlags is FunCall, the other should match because that's
what Warp expects.

Depends on D141617

Comment 6

[Jan de Mooij [:jandem]]

Attachment: Bug 1760605 part 6 - Relax JSOp::FunCall optimization restrictions. r?iain!

Allow other call optimizations at JSOp::FunCall and allow optimizing fun_call
at other call ops.

Depends on D141618

Comment 7

[Jan de Mooij [:jandem]]

Attachment: Bug 1760605 part 7 - Add tests. r?iain!

Depends on D141619

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d87a42f290e6
part 1 - Tidy up InlineFrameIterator::findNextFrame. r=iain
https://hg.mozilla.org/integration/autoland/rev/0ad0428fad88
part 2 - Don't trial inline FunApply{ArgsObj,Array} calls. r=iain
https://hg.mozilla.org/integration/autoland/rev/0e59b292fb48
part 3 - Optimize fun.apply for normal Call ops. r=iain
https://hg.mozilla.org/integration/autoland/rev/e728ed5c9db2
part 4 - Remove JSOp::FunApply. r=iain
https://hg.mozilla.org/integration/autoland/rev/ac9b8297416e
part 5 - Ensure matching JSOp and CallFlags for FunCall. r=iain
https://hg.mozilla.org/integration/autoland/rev/fb669efa4435
part 6 - Relax JSOp::FunCall optimization restrictions. r=iain
https://hg.mozilla.org/integration/autoland/rev/f92971683f01
part 7 - Add tests. r=iain

Comment 9

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/d87a42f290e6
https://hg.mozilla.org/mozilla-central/rev/0ad0428fad88
https://hg.mozilla.org/mozilla-central/rev/0e59b292fb48
https://hg.mozilla.org/mozilla-central/rev/e728ed5c9db2
https://hg.mozilla.org/mozilla-central/rev/ac9b8297416e
https://hg.mozilla.org/mozilla-central/rev/fb669efa4435
https://hg.mozilla.org/mozilla-central/rev/f92971683f01