Closed Bug 1639159 Opened 4 months ago Closed 4 months ago

Optimize ArrayPush in Warp

Categories

(Core :: JavaScript Engine: JIT, task, P3)

task

Tracking

()

RESOLVED FIXED
mozilla78
Tracking Status
firefox78 --- fixed

People

(Reporter: evilpie, Assigned: evilpie)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

No description provided.

Due to the previous guards emitted by CacheIR we know that this is always going to load a static prototype.
For example using MGetPrototypeOf would also try to handle dynamic prototypes.
I added a JIT assertion to make sure this assumption holds.

I tried to compare BaselineCacheIRCompiler::emitArrayPush and CodeGenerator::emitArrayPush to make sure I didn't miss any implicit guards.
It seems like the Ion inline path is actually more limited though.

MArrayPush is typed in Ion to return Int32, that is however not really true, because array.length can overflow INT32_MAX. Ion currently
handles this via TI invalidation. For simplicity I decided to instead add a bailout before we even try to push. In the future we should look into
some approach that doesn't require invalidation to handle this.

Depends on D75925

One question, how would we handle multiple arguments for Array.push?
This used to be a perf hit on Speedometer until Bug 966743.

I haven't really thought about it. Array.push with multiple arguments has been disabled in Ion since two years because of bug 1493903.

Several of the call IC ops read a variable number of arguments from the stack based on argc. In principle, we could do something similar for multiple-argument Array.push.

Alternatively, if it's only the array extension that can fail, and the pushes afterwards are infallible, then we could split ArrayPush into two ops: ArrayExtendN, which fallibly attempts to expand the array, and then a sequence of infallible ArrayPush. That might be easier for Ion, because the argument loads would happen explicitly in CacheIR instead of implicitly inside the op.

Status: NEW → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla78
Regressions: 1653972
You need to log in before you can comment on or make changes to this bug.