Skip to content

[JSC] Implement array-from-async #6937

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.

Sign up for GitHub

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

Jump to bottom
Merged
webkit-early-warning-system merged 1 commit into from
Nov 30, 2022
Merged

[JSC] Implement array-from-async #6937

webkit-early-warning-system merged 1 commit into from
Nov 30, 2022

Conversation

@Constellation
Copy link
Member

@Constellation Constellation commented Nov 29, 2022 •
edited by webkit-early-warning-system
Loading

464a308

[JSC] Implement array-from-async
https://bugs.webkit.org/show_bug.cgi?id=245260
rdar://problem/100303653

Reviewed by Alexey Shvayka.

This patch implements stage-3 Array.fromAsync[1] feature.
The goal of this feature is providing async iteration version of Array.from.

Array.from's concept.

    const arr = [];
    for (const v of iterable) {
        arr.push(v);
    }

Array.fromAsync's concept.

    const arr = [];
    for await (const v of asyncIterable) {
        arr.push(v);
    }

The complicated part of this change is that, when using `async function` in builtin JS,
it automatically generates internal promise, which we would like to avoid here.
In the future, we would like to remove internal promise completely, but for now, we
workaround this restriction by the convention that, when the builtin JS function's name
starts with `defaultAsync`, then we use normal promise instead of internal promise.

[1]: https://github.com/tc39/proposal-array-from-async

* JSTests/stress/array-from-async-basic.js: Added.
(shouldBe):
(shouldBeArray):
(async test.async shouldBeArray):
(async test):
(test.catch):
* JSTests/stress/array-from-async-map-promise.js: Added.
(shouldBe):
(shouldBeArray):
(async test.):
(async test.async shouldBeArray):
(async test):
(test.catch):
* JSTests/stress/array-from-async-map-this.js: Added.
(shouldBe):
(shouldBeArray):
(async test.):
(async test.async shouldBeArray):
(async test):
(test.catch):
* JSTests/stress/array-from-async-map.js: Added.
(shouldBe):
(shouldBeArray):
(async test.):
(async test.async shouldBeArray):
(async test):
(test.catch):
* Source/JavaScriptCore/builtins/ArrayConstructor.js:
(from.wrapper.iterator):
(from):
(wrapper.asyncIterator):
(linkTimeConstant.async defaultAsyncFromAsyncIterator):
(linkTimeConstant.async defaultAsyncFromAsyncArrayLike):
(fromAsync):
* Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:
(JSC::BytecodeGenerator::BytecodeGenerator):
* Source/JavaScriptCore/runtime/ArrayConstructor.cpp:
(JSC::ArrayConstructor::finishCreation):
* Source/JavaScriptCore/runtime/OptionsList.h:

Canonical link: https://commits.webkit.org/257177@main

854b742

Misc iOS, tvOS & watchOS macOS Linux Windows
  🧪 style   🛠 ios   🛠 mac   🛠 wpe   🛠 🧪 win
  🛠 ios-sim   🛠 mac-debug   🛠 gtk   🛠 wincairo
  🧪 webkitperl ❌ 🧪 ios-wk2   🛠 mac-AS-debug ❌ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 api-mac   🧪 api-gtk
  🛠 🧪 jsc   🛠 tv   🧪 mac-wk1   🛠 jsc-armv7
  🛠 tv-sim ❌ 🧪 mac-wk2 ❌ 🧪 jsc-armv7-tests
  🛠 watch   🧪 mac-AS-debug-wk2   🛠 jsc-mips
  🛠 watch-sim ✅ 🧪 mac-wk2-stress ❌ 🧪 jsc-mips-tests

@Constellation Constellation requested a review from a team as a code owner November 29, 2022 21:53
@Constellation Constellation self-assigned this Nov 29, 2022
@Constellation Constellation added the JavaScriptCore For bugs in JavaScriptCore, the JS engine used by WebKit, other than kxmlcore issues. label Nov 29, 2022
@Constellation Constellation force-pushed the eng/JSC-Implement-array-from-async branch from f46443c to 100a843 Compare November 29, 2022 21:54
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Nov 29, 2022 •
edited
Loading

EWS run on previous version of this PR (hash 100a843)

Details

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ⏳ 🛠 mac ✅ 🛠 wpe ⏳ 🛠 🧪 win
✅ 🛠 ios-sim ⏳ 🛠 mac-debug ⏳ 🛠 gtk ⏳ 🛠 wincairo
✅ 🧪 webkitperl   🧪 ios-wk2 ⏳ 🛠 mac-AS-debug ⏳ 🧪 gtk-wk2
  🧪 api-ios ⏳ 🧪 api-mac ⏳ 🧪 api-gtk
⏳ 🛠 🧪 jsc ⏳ 🛠 tv ⏳ 🧪 mac-wk1 ✅ 🛠 jsc-armv7
⏳ 🛠 tv-sim ⏳ 🧪 mac-wk2   🧪 jsc-armv7-tests
✅ 🛠 watch ⏳ 🧪 mac-AS-debug-wk2 ✅ 🛠 jsc-mips
✅ 🛠 watch-sim ⏳ 🧪 mac-wk2-stress   🧪 jsc-mips-tests

@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Nov 29, 2022 •
edited
Loading

EWS run on previous version of this PR (hash f46443c)

Details

Misc iOS, tvOS & watchOS macOS Linux Windows
⏳ 🧪 style ✅ 🛠 ios ⏳ 🛠 mac ⏳ 🛠 wpe ⏳ 🛠 🧪 win
⏳ 🧪 bindings   🛠 ios-sim ⏳ 🛠 mac-debug ⏳ 🛠 gtk ⏳ 🛠 wincairo
⏳ 🧪 webkitperl   🧪 ios-wk2 ⏳ 🛠 mac-AS-debug ⏳ 🧪 gtk-wk2
⏳ 🧪 webkitpy   🧪 api-ios ⏳ 🧪 api-mac ⏳ 🧪 api-gtk
⏳ 🛠 🧪 jsc ⏳ 🛠 tv ⏳ 🧪 mac-wk1   🛠 jsc-armv7
⏳ 🧪 services ⏳ 🛠 tv-sim ⏳ 🧪 mac-wk2   🧪 jsc-armv7-tests
⏳ 🛠 watch ⏳ 🧪 mac-AS-debug-wk2   🛠 jsc-mips
⏳ 🛠 watch-sim ⏳ 🧪 mac-wk2-stress   🧪 jsc-mips-tests

shvaikalesh
shvaikalesh approved these changes Nov 29, 2022
View reviewed changes
Source/JavaScriptCore/runtime/ArrayConstructor.cpp Outdated
Copy link
Member

@shvaikalesh shvaikalesh Nov 29, 2022

Choose a reason for hiding this comment

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

This isn't actually necessary as the length of JSBuiltin functions is deducted from their signature. Covered by test262.

Copy link
Member Author

@Constellation Constellation Nov 29, 2022

Choose a reason for hiding this comment

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

Yes, but anyway, I just fixed it since it is wrong :)

shvaikalesh
shvaikalesh requested changes Nov 29, 2022
View reviewed changes
@Constellation Constellation force-pushed the eng/JSC-Implement-array-from-async branch from 100a843 to caf7d4c Compare November 29, 2022 22:19
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Nov 29, 2022

Starting EWS tests for caf7d4c. Live statuses available at the PR page, #6937

@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Nov 29, 2022 •
edited
Loading

EWS run on previous version of this PR (hash caf7d4c)

Details

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style   🛠 ios ⏳ 🛠 mac ✅ 🛠 wpe ⏳ 🛠 🧪 win
  🛠 ios-sim ⏳ 🛠 mac-debug ⏳ 🛠 gtk ⏳ 🛠 wincairo
✅ 🧪 webkitperl   🧪 ios-wk2 ⏳ 🛠 mac-AS-debug ⏳ 🧪 gtk-wk2
  🧪 api-ios ⏳ 🧪 api-mac ⏳ 🧪 api-gtk
⏳ 🛠 🧪 jsc ⏳ 🛠 tv ⏳ 🧪 mac-wk1 ⏳ 🛠 jsc-armv7
⏳ 🛠 tv-sim ⏳ 🧪 mac-wk2 ⏳ 🧪 jsc-armv7-tests
⏳ 🛠 watch ⏳ 🧪 mac-AS-debug-wk2 ✅ 🛠 jsc-mips
⏳ 🛠 watch-sim ⏳ 🧪 mac-wk2-stress   🧪 jsc-mips-tests

@Constellation Constellation force-pushed the eng/JSC-Implement-array-from-async branch from caf7d4c to 9c02c78 Compare November 29, 2022 22:22
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Nov 29, 2022 •
edited
Loading

EWS run on previous version of this PR (hash 9c02c78)

Details

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ⏳ 🛠 ios ⏳ 🛠 mac ⏳ 🛠 wpe ⏳ 🛠 🧪 win
  🛠 ios-sim ⏳ 🛠 mac-debug ⏳ 🛠 gtk ⏳ 🛠 wincairo
✅ 🧪 webkitperl   🧪 ios-wk2 ⏳ 🛠 mac-AS-debug ⏳ 🧪 gtk-wk2
  🧪 api-ios ⏳ 🧪 api-mac ⏳ 🧪 api-gtk
⏳ 🛠 🧪 jsc ⏳ 🛠 tv ⏳ 🧪 mac-wk1 ✅ 🛠 jsc-armv7
⏳ 🛠 tv-sim ⏳ 🧪 mac-wk2   🧪 jsc-armv7-tests
  🛠 watch ⏳ 🧪 mac-AS-debug-wk2 ✅ 🛠 jsc-mips
⏳ 🛠 watch-sim ⏳ 🧪 mac-wk2-stress   🧪 jsc-mips-tests

@Constellation Constellation force-pushed the eng/JSC-Implement-array-from-async branch from 9c02c78 to 854b742 Compare November 29, 2022 22:29
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Nov 29, 2022 •
edited
Loading

EWS run on current version of this PR (hash 854b742)

Details

Misc iOS, tvOS & watchOS macOS Linux Windows
  🧪 style   🛠 ios   🛠 mac   🛠 wpe   🛠 🧪 win
  🛠 ios-sim   🛠 mac-debug   🛠 gtk   🛠 wincairo
  🧪 webkitperl ❌ 🧪 ios-wk2   🛠 mac-AS-debug ❌ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 api-mac   🧪 api-gtk
  🛠 🧪 jsc   🛠 tv   🧪 mac-wk1   🛠 jsc-armv7
  🛠 tv-sim ❌ 🧪 mac-wk2 ❌ 🧪 jsc-armv7-tests
  🛠 watch   🧪 mac-AS-debug-wk2   🛠 jsc-mips
  🛠 watch-sim ✅ 🧪 mac-wk2-stress ❌ 🧪 jsc-mips-tests

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Nov 30, 2022
@Constellation Constellation removed the merging-blocked Applied to prevent a change from being merged label Nov 30, 2022
@Constellation Constellation force-pushed the eng/JSC-Implement-array-from-async branch from 854b742 to 8329ff0 Compare November 30, 2022 14:54
@Constellation Constellation added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Nov 30, 2022
@Constellation
[JSC] Implement array-from-async
464a308 
https://bugs.webkit.org/show_bug.cgi?id=245260
rdar://problem/100303653

Reviewed by Alexey Shvayka.

This patch implements stage-3 Array.fromAsync[1] feature.
The goal of this feature is providing async iteration version of Array.from.

Array.from's concept.

    const arr = [];
    for (const v of iterable) {
        arr.push(v);
    }

Array.fromAsync's concept.

    const arr = [];
    for await (const v of asyncIterable) {
        arr.push(v);
    }

The complicated part of this change is that, when using `async function` in builtin JS,
it automatically generates internal promise, which we would like to avoid here.
In the future, we would like to remove internal promise completely, but for now, we
workaround this restriction by the convention that, when the builtin JS function's name
starts with `defaultAsync`, then we use normal promise instead of internal promise.

[1]: https://github.com/tc39/proposal-array-from-async

* JSTests/stress/array-from-async-basic.js: Added.
(shouldBe):
(shouldBeArray):
(async test.async shouldBeArray):
(async test):
(test.catch):
* JSTests/stress/array-from-async-map-promise.js: Added.
(shouldBe):
(shouldBeArray):
(async test.):
(async test.async shouldBeArray):
(async test):
(test.catch):
* JSTests/stress/array-from-async-map-this.js: Added.
(shouldBe):
(shouldBeArray):
(async test.):
(async test.async shouldBeArray):
(async test):
(test.catch):
* JSTests/stress/array-from-async-map.js: Added.
(shouldBe):
(shouldBeArray):
(async test.):
(async test.async shouldBeArray):
(async test):
(test.catch):
* Source/JavaScriptCore/builtins/ArrayConstructor.js:
(from.wrapper.iterator):
(from):
(wrapper.asyncIterator):
(linkTimeConstant.async defaultAsyncFromAsyncIterator):
(linkTimeConstant.async defaultAsyncFromAsyncArrayLike):
(fromAsync):
* Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:
(JSC::BytecodeGenerator::BytecodeGenerator):
* Source/JavaScriptCore/runtime/ArrayConstructor.cpp:
(JSC::ArrayConstructor::finishCreation):
* Source/JavaScriptCore/runtime/OptionsList.h:

Canonical link: https://commits.webkit.org/257177@main
@webkit-early-warning-system webkit-early-warning-system force-pushed the eng/JSC-Implement-array-from-async branch from 8329ff0 to 464a308 Compare November 30, 2022 15:01
@webkit-commit-queue
Copy link
Collaborator

webkit-commit-queue commented Nov 30, 2022

Committed 257177@main (464a308): https://commits.webkit.org/257177@main

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

@webkit-early-warning-system webkit-early-warning-system merged commit 464a308 into WebKit:main Nov 30, 2022
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Nov 30, 2022
@Constellation Constellation deleted the eng/JSC-Implement-array-from-async branch November 30, 2022 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@shvaikalesh shvaikalesh shvaikalesh approved these changes

Assignees

@Constellation Constellation

Labels

JavaScriptCore For bugs in JavaScriptCore, the JS engine used by WebKit, other than kxmlcore issues.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Constellation @webkit-early-warning-system @webkit-commit-queue @shvaikalesh @webkit-ews-buildbot