1666009 - Increase maximum bytecode size when trial inlining self-hosted functions

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, enhancement, P2)

Description

[Tom S [:evilpie]]

The current maximum bytecode size is too small for many self-hosted functions. For example we aren't inlining ArrayIteratorNext. We also aren't inlining the much smaller ArrayItem method that I am adding in bug 1658308.

Comment 1

[Nicolas B. Pierron [:nbp | PTO until 11/11]]

Another option would be to make it depend on the context of the caller.
For example, to improve map/forOf we can suggest to inline it by the fact that this is a lambda given as argument.

The same could be done for the next function call, if we can identify that this is an iterator pattern.

Comment 2

[Tom S [:evilpie]]

Attachment: Bug 1666009 - Make it possible to mark large self-hosted functions as inlinable

Right now 'force inlined' just means 'ignore the max function size'. This seems to be the most common reason why we don't inline.
To other conditions TrialInliner::shouldInline still seem useful or necessary to me.
The name 'isForceInlining' is a bit awkward maybe?

This reduces the Array#at runtime from 200ms to 150ms in a micro-benchmark. I plan on adding this to some other important
functions like .map etc.

Comment 3

[Nicolas B. Pierron [:nbp | PTO until 11/11]]

Looking at Array#at, it sounds to me that when the function is used, it would mostly be using a single code path, while having no loop. Could we synthesize this an an heuristic, which ignore the function size or make the function size irrelevant to some larger extend?

My point of view about adding a special function to force the inlining of self-hosted functions is that we should avoid as much as possible, as these are explicit failures at optimizing user code as much as we want our self-hosted code to be optimized.

Comment 4

[Iain Ireland [:iain]]

In a perfect world, we would have a smart heuristic that correctly identifies good inlining candidates. But we're a long way from there. Right now we have a dumb heuristic that aggressively rules out any inlining candidates it thinks might be bad.

This approach will make our inlining decisions better for now. It will also let us experiment with manual heuristics about which self-hosted functions should be marked, which can inform our design of automated heuristics in the future.

(Also, I think it's sometimes reasonable to optimize self-hosted code differently. We have AOT information about which paths are expected to be hot/cold in self-hosted code that isn't freely available for user code. We could track it at execution time for user code, but that imposes overhead relative to the precomputation here.)

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/eaff5f16e41c
Make it possible to mark large self-hosted functions as inlinable r=iain,tcampbell

Comment 6

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/eaff5f16e41c