Support JSOp::Resume in Warp
Categories
(Core :: JavaScript Engine: JIT, task)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox155 | --- | fixed |
People
(Reporter: jandem, Assigned: jandem)
References
(Blocks 1 open bug)
Details
Attachments
(10 files)
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
Bug 2060131 part 7 - Rename CallIsSuspendedGeneratorResult op to IsSuspendedGeneratorResult. r?iain!
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review |
Bug 2058411 changed generator-resume to be callee-driven and JSOp::Resume is now similar to a function call. This means we can now support it in WarpBuilder. This lets us Ion-compile + inline the self-hosted GeneratorNext function.
This is just the resumer side; Ion-compiling the generator/async function itself is more complicated and is the bigger win, but this is also a small improvement: the micro-benchmark below improves from 1838 ms to 1604 ms on my machine.
function* gen() {
for (var i = 0; i < 100_000_000; i++) {
yield 1;
}
}
function f() {
var res = 0;
var t = Date.now();
for (var x of gen()) {
res += x;
}
print(Date.now() - t);
print(res);
}
f();
|
Assignee | |
Comment 1•4 days ago
|
||
This adds a fourth slot to ResumeFrameArgs for the generator's resume index.
The resumer is now responsible for reading the generator's resume index slot and
marking the generator as running (this clobbers the generator's resume index slot).
Initially this was required for part 4, but I've since changed that path to no longer
allow executing arbitrary JS through the interrupt handler. However having the resume
index in ResumeFrameArgs also simplifies the Ion implementation later on and will be
more efficient in some cases, so I've kept it.
|
|
Assignee | |
Comment 2•4 days ago
|
||
The next patch will use this to restore registers in emitIsDebuggeeCheck only if
we actually called into C++.
|
|
Assignee | |
Comment 3•4 days ago
|
||
We were doing this as part of JSOp::AfterYield, but that's too late when we
add an overrecursion check to the resume prologue.
Setting the flag in the prologue makes this consistent with both the C++ interpreter's
resume code and Baseline's non-resume prologue.
|
|
Assignee | |
Comment 4•4 days ago
|
||
This is more consistent with the normal (non-resume) prologue. Currently all
generator-resumes go through GeneratorNext and it has an overrecursion check,
but when Ion can inline that function and compile generators, the generator's
own overrecursion check becomes more important.
Unlike the normal overrecursion check in the prologue, this one does not handle
interrupts. Supporting interrupts while we're in the middle of resuming a generator
frame is complicated considering Debugger interactions.
|
|
Assignee | |
Comment 5•4 days ago
|
||
GeneratorNext now has a bytecode length of ~115, less than smallFunctionMaxBytecodeLength (140).
This ensures we can inline it once WarpBuilder can compile JSOp::Resume.
The GeneratorSetClosed intrinsic was only called in the catch-blocks, after
checking !GeneratorObjectIsClosed. Move that if-statement into the intrinsic to
avoid the extra call and branch.
|
|
Assignee | |
Comment 6•4 days ago
|
||
Now that JSOp::Resume is very similar to a call, we can Ion-compile and inline
the self-hosted GeneratorNext function. (GeneratorThrow and GeneratorReturn
can also be Ion-compiled now but they're less performance-sensitive.)
|
|
Assignee | |
Comment 7•4 days ago
|
||
This op doesn't make a call.
|
|
Assignee | |
Comment 8•4 days ago
|
||
This will avoid a redundant unbox in Ion.
|
|
Assignee | |
Comment 9•4 days ago
|
||
This is required to get good performance for Ion-compiled GeneratorNext.
|
|
Assignee | |
Comment 10•4 days ago
|
||
BaselineFrame::trace and InterpreterFrame::trace clear the locals that aren't live
at the frame's pc. Nothing between restoring these slots and setting the pc can GC,
so assert this to prevent a (future) footgun.
This was an issue for an earlier prototype based on a different approach. It caused
JetStream to fail intermittently.
|
||
Comment 11•3 days ago
|
||
|
||
Comment 12•3 days ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/f8944199b9e5
https://hg.mozilla.org/mozilla-central/rev/4256d4d5e006
https://hg.mozilla.org/mozilla-central/rev/8b06c0d9e6a0
https://hg.mozilla.org/mozilla-central/rev/c008707e2e86
https://hg.mozilla.org/mozilla-central/rev/c8c48b313662
https://hg.mozilla.org/mozilla-central/rev/0b3651bc33f5
https://hg.mozilla.org/mozilla-central/rev/676585a0fe2f
https://hg.mozilla.org/mozilla-central/rev/3b44c81dae61
https://hg.mozilla.org/mozilla-central/rev/10f0f7db2d8a
https://hg.mozilla.org/mozilla-central/rev/ce8346d5a287
Description
•