Closed Bug 1666009 Opened 4 years ago Closed 4 years ago

Increase maximum bytecode size when trial inlining self-hosted functions

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
86 Branch
Tracking Status
firefox86 --- fixed

People

(Reporter: evilpie, Assigned: evilpie)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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.

Summary: Increase maximum function size when trial inlining self-hosted functions → Increase maximum bytecode size when trial inlining self-hosted functions

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.

Severity: -- → N/A
Type: task → enhancement
Priority: -- → P2

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.

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.

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.)

Assignee: nobody → evilpies
Attachment #9196277 - Attachment description: Bug 1666009 - [WIP] Make it possible to mark self-hosted functions as 'force inlined' → Bug 1666009 - Make it possible to mark large self-hosted functions as inlinable
Blocks: 1687025
Pushed by evilpies@gmail.com: https://hg.mozilla.org/integration/autoland/rev/eaff5f16e41c Make it possible to mark large self-hosted functions as inlinable r=iain,tcampbell
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 86 Branch
Regressions: 1687159
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: