Skip to content

Experimental async/await support#112

Merged
kateinoigakukun merged 23 commits intonextfrom
katei/async-promise
Jan 13, 2021
Merged

Experimental async/await support#112
kateinoigakukun merged 23 commits intonextfrom
katei/async-promise

Conversation

@kateinoigakukun
Copy link
Member

@kateinoigakukun kateinoigakukun commented Jan 2, 2021 •
edited
Loading 

import JavaScriptEventLoop
import JavaScriptKit

JavaScriptEventLoop.install()
let fetch = JSObject.global.fetch.function!.async

func printZen() async {
  let result = await try! fetch("https://api.github.com/zen").object!
  let text = await try! result.asyncing.text!()
  print(text)
}

JavaScriptEventLoop.runAsync {
  await printZen()
}

Current Issues

  • Many C++ headers are copied from LLVM and Swift repository even though the runtime uses only a few interfaces.
  • No tests
  • Use of unsafe flag -enable-experimental-concurrency. If we remove it from Package.swift, users have to pass it from command line.

j-f1
j-f1 reviewed Jan 2, 2021
View reviewed changes
@github-actions
Copy link

github-actions bot commented Jan 11, 2021 •
edited
Loading

Time Change: -843ms (9%) ✅

Total Time: 9,170.5ms

Test name Duration Change
Serialization/Swift Int to JavaScript 2,759.75ms -455.25ms (16%) 👏
Serialization/Swift String to JavaScript 2,966ms -284.75ms (9%) ✅
ℹ️ View Unchanged
Test name Duration Change
Serialization/Write JavaScript number directly 190.75ms +1.25ms (0%)
Serialization/Write JavaScript string directly 198.5ms +4.25ms (2%)
Object heap/Increment and decrement RC 3,055.5ms -108.5ms (3%)

performance-action

@kateinoigakukun kateinoigakukun changed the base branch from main to next January 11, 2021 05:44
@kateinoigakukun kateinoigakukun changed the title [Do not merge] Experimental async/await support Experimental async/await support Jan 11, 2021
@kateinoigakukun kateinoigakukun marked this pull request as ready for review January 11, 2021 05:44
@MaxDesiatov
Copy link
Member

MaxDesiatov commented Jan 11, 2021

Do I understand correctly that after this is merged we'll require our users to use dev toolchains and to compile with experimental concurrency enabled?

@MaxDesiatov
Copy link
Member

MaxDesiatov commented Jan 11, 2021

Also, could we link with Swift runtime parts that are already included in the toolchain/SDK? Is a copy of it and LLVM stuff in our codebase required for this to work?

@kateinoigakukun
Copy link
Member Author

kateinoigakukun commented Jan 13, 2021 •
edited
Loading

Do I understand correctly that after this is merged we'll require our users to use dev toolchains and to compile with experimental concurrency enabled?

Yes, so I'm planning to merge this to next branch which will be used to use non-stable language features.

Also, could we link with Swift runtime parts that are already included in the toolchain/SDK? Is a copy of it and LLVM stuff in our codebase required for this to work?

JavaScriptEventLoop should be linked with libswiftCore.a and libswift_Concurrency.a to work. And LLVM headers are required for including swift/ABI/Task.h.

@MaxDesiatov
Copy link
Member

MaxDesiatov commented Jan 13, 2021

Fair enough. I'm only worried that contents of these headers can change in a breaking way, especially given that concurrency stuff isn't ABI-stable yet, and maybe there's a better way to track them. A submodule is probably not practical due to the size of repositories that have these headers, so I don't have a better alternative to propose... Maybe some kind of scripts that pull latest versions of these headers automatically? 🤔

@kateinoigakukun
Copy link
Member Author

kateinoigakukun commented Jan 13, 2021 •
edited
Loading

Yes, so I wrote a shell script to fetch headers from swiftwasm/swift repository based on .swift-version snapshot.

https://github.com/swiftwasm/JavaScriptKit/blob/366d74e82e3aa40d604ac843743bb59b9412243b/Sources/_CJavaScriptEventLoop/README
https://github.com/swiftwasm/JavaScriptKit/blob/366d74e82e/Sources/_CJavaScriptEventLoop/scripts/copy-headers

MaxDesiatov
MaxDesiatov approved these changes Jan 13, 2021
View reviewed changes
Copy link
Member

@MaxDesiatov MaxDesiatov left a comment

Choose a reason for hiding this comment

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

Seems legit then 👍

@kateinoigakukun kateinoigakukun merged commit d164af8 into next Jan 13, 2021
@kateinoigakukun kateinoigakukun deleted the katei/async-promise branch January 13, 2021 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@j-f1 j-f1 j-f1 left review comments

@MaxDesiatov MaxDesiatov MaxDesiatov approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@kateinoigakukun @MaxDesiatov @j-f1

Comments