Bugzilla

After a few weeks on this, I've got a working patch (on Phab) that (i) [saves ~3% of JS heap on AWSY tests](https://treeherder.mozilla.org/perf.html#/compare?originalProject=mozilla-central&newProject=try&newRevision=a09ac2ebaa5de7cfdaa87845bc46f85bc54ed535&framework=4&selectedTimeRange=172800), but (ii) [slows down Speedometer by ~2%](https://treeherder.mozilla.org/perf.html#/compare?originalProject=mozilla-central&newProject=try&newRevision=a09ac2ebaa5de7cfdaa87845bc46f85bc54ed535&framework=10&selectedTimeRange=172800). The latter is pretty persistent -- I spent the past few weeks reworking things to have a fallback path rather than bailout, and it's still a perf loss -- so it's looking like this may be the best we get with this approach.

Thoughts on this tradeoff? Take it for Fission maybe?

The patch does the following:

* Removes separate groups for all non-singleton functions.
* In baseline IC attach log ic for call ops, emits a meta-op that records the `JSScript` of the called function.
* At Ion compilation time, when deciding whether/what to inline, looks at the `TypeSet` for the callee as normal. If nothing, or if the general `Function` group corresponding to non-singleton functions is present in the set, then inspect the baseline IC chain to find this meta-op. If present, the set of possible targets contains this *script*. The `InliningTarget` type is now a three-way variant: singleton function, callable non-function object, or non-singleton function identified by script / canonical function.
* If inlining a target identified by script only, use a new MIR op `MTestScript` that tests the script pointer of a given function object against a baked-in expected value. If match, branch to inlined path. If no match, branch to fallback path that does a normal call.

There were a *lot* of subtle bugs to resolve here, once I got past the basic "how does IonBuilder work" (understanding the stack invariants, how to patch up resume points to avoid mysterious appearances of Magic(optimized-out) values, etc). In particular, the Definite Properties Analysis can no longer analyze a function that calls non-singleton functions, because it relies on constraints that freeze the observed inlining (and we can't use constraints if we don't have groups!). There's also some interesting interaction with object-format assumptions -- if we defer the function target test to runtime (rather than constraint-based), then e.g. the load-function-environment op cannot be lifted above the `MTestScript` because the function may even be a native function, so I had to add a new category to the alias analysis to fix that. In summary, all tests are green now, but this needs some careful eyeballs to look at it.

After a few weeks on this, I've got a working patch (on Phab) that (i) [saves ~3% of JS heap on AWSY tests](https://treeherder.mozilla.org/perf.html#/compare?originalProject=mozilla-central&newProject=try&newRevision=a09ac2ebaa5de7cfdaa87845bc46f85bc54ed535&framework=4&selectedTimeRange=172800), but (ii) [slows down Speedometer by ~2%](https://treeherder.mozilla.org/perf.html#/compare?originalProject=mozilla-central&newProject=try&newRevision=a09ac2ebaa5de7cfdaa87845bc46f85bc54ed535&framework=10&selectedTimeRange=172800). The latter is pretty persistent -- I spent the past few weeks reworking things to have a fallback path rather than bailout, and it's still a perf loss -- so it's looking like this may be the best we get with this approach.

Thoughts on this tradeoff? Take it for Fission maybe?

The patch does the following:

* Removes separate groups for all non-singleton functions.
* In baseline IC attach logic for call ops, emits a meta-op that records the `JSScript` of the called function.
* At Ion compilation time, when deciding whether/what to inline, looks at the `TypeSet` for the callee as normal. If nothing, or if the general `Function` group corresponding to non-singleton functions is present in the set, then inspect the baseline IC chain to find this meta-op. If present, the set of possible targets contains this *script*. The `InliningTarget` type is now a three-way variant: singleton function, callable non-function object, or non-singleton function identified by script / canonical function.
* If inlining a target identified by script only, use a new MIR op `MTestScript` that tests the script pointer of a given function object against a baked-in expected value. If match, branch to inlined path. If no match, branch to fallback path that does a normal call.

There were a *lot* of subtle bugs to resolve here, once I got past the basic "how does IonBuilder work" (understanding the stack invariants, how to patch up resume points to avoid mysterious appearances of Magic(optimized-out) values, etc). In particular, the Definite Properties Analysis can no longer analyze a function that calls non-singleton functions, because it relies on constraints that freeze the observed inlining (and we can't use constraints if we don't have groups!). There's also some interesting interaction with object-format assumptions -- if we defer the function target test to runtime (rather than constraint-based), then e.g. the load-function-environment op cannot be lifted above the `MTestScript` because the function may even be a native function, so I had to add a new category to the alias analysis to fix that. In summary, all tests are green now, but this needs some careful eyeballs to look at it.