Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JSC] Async / Await should not wrap awaited value with one tick #1143

Merged
merged 0 commits into from
May 29, 2022

Conversation

Constellation
Copy link
Member

@Constellation Constellation commented May 29, 2022

fb3470e

[JSC] Async / Await should not wrap awaited value with one tick
https://bugs.webkit.org/show_bug.cgi?id=241072

Reviewed by Saam Barati.

This patch integrates spec change[1], which removes one level tick count when resolving promise with await.
Previously, regardless of whether the value is promise or not, we are always using resolveWithoutPromise,
but it introduces one tick before the handlers are resolved. The spec change makes it that we can call
performPromiseThen directly if the input value is promise, so we can skip one tick which looks up "then"
and register handlers.

This is beneficial for await performance and it also fixes a bug tested via test262 and attached test due
to the spec change.

We observed performance improvement in async + native promise tests.

    ToT
    Time(doxbee-async-es2017-native): 35.6 ms.
    Time(fibonacci-async-es2017-native): 292.3 ms.
    Time(parallel-async-es2017-native): 117.3 ms.

    Patched
    Time(doxbee-async-es2017-native): 24.2 ms.
    Time(fibonacci-async-es2017-native): 198.1 ms.
    Time(parallel-async-es2017-native): 109.5 ms.

[1]: tc39/ecma262#1250

* JSTests/stress/async-await-basic.js:
* JSTests/stress/async-await-tick-count.js: Added.
(shouldBe):
(async returnDirectPrimitive):
(async returnAwaitPrimitive):
(async returnDirectPromisePrimitive):
(async returnAwaitPromisePrimitive):
(async test):
(async tests):
(globalThis.setUnhandledRejectionCallback.setUnhandledRejectionCallback):
* JSTests/test262/expectations.yaml:
* LayoutTests/inspector/canvas/recording-bitmaprenderer-frameCount-expected.txt:
* LayoutTests/inspector/canvas/recording-bitmaprenderer-full-expected.txt:
* LayoutTests/inspector/canvas/recording-bitmaprenderer-memoryLimit-expected.txt:
* LayoutTests/inspector/console/message-stack-trace-expected.txt:
* Source/JavaScriptCore/builtins/AsyncFromSyncIteratorPrototype.js:
* Source/JavaScriptCore/builtins/AsyncFunctionPrototype.js:
(globalPrivate.asyncFunctionResume):
* Source/JavaScriptCore/builtins/AsyncGeneratorPrototype.js:
(globalPrivate.awaitValue):
(globalPrivate.asyncGeneratorResumeNext):
* Source/JavaScriptCore/builtins/PromiseOperations.js:
(globalPrivate.newPromiseCapabilitySlow):
(globalPrivate.promiseResolve):
(globalPrivate.promiseResolveSlow):
(globalPrivate.promiseRejectSlow):
(globalPrivate.resolvePromiseWithFirstResolvingFunctionCallCheck):
(globalPrivate.fulfillPromiseWithFirstResolvingFunctionCallCheck):
(globalPrivate.rejectPromiseWithFirstResolvingFunctionCallCheck):
(globalPrivate.resolveWithoutPromiseForAsyncAwait):

Canonical link: https://commits.webkit.org/251106@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@295011 268f45cc-cd09-0410-ab3c-d52691b4dbfc

@Constellation Constellation self-assigned this May 29, 2022
@Constellation Constellation added JavaScriptCore For bugs in JavaScriptCore, the JS engine used by WebKit, other than kxmlcore issues. WebKit Nightly Build labels May 29, 2022
Copy link
Contributor

@saambarati saambarati left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r=me

@webkit-early-warning-system webkit-early-warning-system added the merging-blocked Applied to prevent a change from being merged label May 29, 2022
@Constellation Constellation removed the merging-blocked Applied to prevent a change from being merged label May 29, 2022
@webkit-early-warning-system webkit-early-warning-system added the merging-blocked Applied to prevent a change from being merged label May 29, 2022
@Constellation Constellation removed merging-blocked Applied to prevent a change from being merged JavaScriptCore For bugs in JavaScriptCore, the JS engine used by WebKit, other than kxmlcore issues. WebKit Nightly Build labels May 29, 2022
@Constellation Constellation added JavaScriptCore For bugs in JavaScriptCore, the JS engine used by WebKit, other than kxmlcore issues. WebKit Nightly Build merge-queue Applied to send a pull request to merge-queue labels May 29, 2022
@webkit-early-warning-system webkit-early-warning-system merged commit fb3470e into WebKit:main May 29, 2022
@webkit-early-warning-system
Copy link
Collaborator

Committed r295011 (251106@main): https://commits.webkit.org/251106@main

Reviewed commits have been landed. Closing PR #1143 and removing active labels.

@webkit-early-warning-system webkit-early-warning-system removed the merge-queue Applied to send a pull request to merge-queue label May 29, 2022
@Constellation Constellation deleted the eng/reducetick branch May 29, 2022 11:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
JavaScriptCore For bugs in JavaScriptCore, the JS engine used by WebKit, other than kxmlcore issues.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants