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

Convert createWasm async/await where possible #23157

Merged
merged 1 commit into from
Dec 19, 2024

Conversation

sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Dec 13, 2024

The advantage if using await in the cases where we can is that it avoids the generation or wrapper function for each export.

So instead of

var wasmExport = createWasm(); // returns empty object
...
var malloc = (..) => (malloc = wasmExports['malloc'])(..);

We can just generate:

var wasmExport = await createWasm(); // returns actual exports
...
var malloc = wasmExports['malloc'];

This only currently works in MODULARIZE mode where the code is running inside a factory function. Otherwise it would end up using top-level-await.

One wrinkle here is that this is not currently supported when closure is enabled because we run closure only on the contents of the factory function so closure ends up seeing this as a top level await when its not.

There are two minor observable effects of this change:

  1. In MODULARIZE mode its no longer possible to call new Foo() with factory function. We already added a warning for this debug builds in Assert if MODULARIZE factory function is used with new. #23210.
  2. Because we now can await for createWasm to return, the run method can run on first call, which means it runs main on first call, which means main will now run before postjs code, just like it would with sync instantiation.

sbc100 added a commit to sbc100/emscripten that referenced this pull request Dec 16, 2024
This is a debug feature that means users who did not await the promise
and tried to access stuff directly on it would see a nice error message.

However with the advent of more usage of async/await, for example in
\emscripten-core#23157 it is not always the ready promise that gets returned to the
user but a language-generated promise from an `await` call.

We could try to jump through more hoops to continue to support this
debug feature, but I don't think added complexity in the toolchain
or the generated code would be worth it.

Split out from emscripten-core#23157.
sbc100 added a commit to sbc100/emscripten that referenced this pull request Dec 16, 2024
This is a debug feature that means users who did not await the promise
and tried to access stuff directly on it would see a nice error message.

However with the advent of more usage of async/await, for example in
\emscripten-core#23157 it is not always the ready promise that gets returned to the
user but a language-generated promise from an `await` call.

We could try to jump through more hoops to continue to support this
debug feature, but I don't think added complexity in the toolchain
or the generated code would be worth it.

Split out from emscripten-core#23157.
sbc100 added a commit that referenced this pull request Dec 16, 2024
This is a debug feature that means users who did not await the promise
and tried to access stuff directly on it would see a nice error message.

However with the advent of more usage of async/await, for example in
\#23157 it is not always the ready promise that gets returned to the
user but a language-generated promise from an `await` call.

We could try to jump through more hoops to continue to support this
debug feature, but I don't think added complexity in the toolchain or
the generated code would be worth it.

Split out from #23157.
@sbc100 sbc100 force-pushed the await_createWasm branch 6 times, most recently from b85a441 to 3106ca5 Compare December 17, 2024 21:10
test/test_browser.py Outdated Show resolved Hide resolved
@sbc100 sbc100 requested a review from dschuff December 17, 2024 22:26
@sbc100 sbc100 changed the title Allow the createWasm function to use async/await where possible Convert createWasm async/await where possible Dec 17, 2024
@sbc100 sbc100 changed the title Convert createWasm async/await where possible Convert createWasm async/await where possible Dec 17, 2024
test/test_browser.py Outdated Show resolved Hide resolved
tools/emscripten.py Show resolved Hide resolved
sbc100 added a commit to sbc100/emscripten that referenced this pull request Dec 18, 2024
Once we start using `async` for for this function then `new` stops
working: emscripten-core#23157.

Using `new` in this was was always an antipattern but there was nothing
stopping users from doing this.
sbc100 added a commit to sbc100/emscripten that referenced this pull request Dec 18, 2024
Once we start using `async` for for this function then `new` stops
working: emscripten-core#23157.

Using `new` in this was was always an antipattern but there was nothing
stopping users from doing this.
sbc100 added a commit to sbc100/emscripten that referenced this pull request Dec 18, 2024
Once we start using `async` for for this function then `new` stops
working: emscripten-core#23157.

Using `new` in this was was always an antipattern but there was nothing
stopping users from doing this.
sbc100 added a commit to sbc100/emscripten that referenced this pull request Dec 18, 2024
Once we start using `async` for for this function then `new` stops
working: emscripten-core#23157.

Using `new` in this was was always an antipattern but there was nothing
stopping users from doing this.
sbc100 added a commit to sbc100/emscripten that referenced this pull request Dec 18, 2024
Once we start using `async` for for this function then `new` stops
working: emscripten-core#23157.

Using `new` in this was was always an antipattern but there was nothing
stopping users from doing this.
hedwigz pushed a commit to hedwigz/emscripten that referenced this pull request Dec 18, 2024
This is a debug feature that means users who did not await the promise
and tried to access stuff directly on it would see a nice error message.

However with the advent of more usage of async/await, for example in
\emscripten-core#23157 it is not always the ready promise that gets returned to the
user but a language-generated promise from an `await` call.

We could try to jump through more hoops to continue to support this
debug feature, but I don't think added complexity in the toolchain or
the generated code would be worth it.

Split out from emscripten-core#23157.
sbc100 added a commit to sbc100/emscripten that referenced this pull request Dec 18, 2024
Once we start using `async` for for this function then `new` stops
working: emscripten-core#23157.

Using `new` in this was was always an antipattern but there was nothing
stopping users from doing this.
sbc100 added a commit to sbc100/emscripten that referenced this pull request Dec 18, 2024
Once we start using `async` for for this function then `new` stops
working: emscripten-core#23157.

Using `new` in this was was always an antipattern but there was nothing
stopping users from doing this.
sbc100 added a commit to sbc100/emscripten that referenced this pull request Dec 18, 2024
Once we start using `async` for for this function then `new` stops
working: emscripten-core#23157.

Using `new` in this was was always an antipattern but there was nothing
stopping users from doing this.
sbc100 added a commit to sbc100/emscripten that referenced this pull request Dec 18, 2024
Once we start using `async` for for this function then `new` stops
working: emscripten-core#23157.

Using `new` in this was was always an antipattern but there was nothing
stopping users from doing this.
sbc100 added a commit to sbc100/emscripten that referenced this pull request Dec 18, 2024
Once we start using `async` for for this function then `new` stops
working: emscripten-core#23157.

Using `new` in this was was always an antipattern but there was nothing
stopping users from doing this.
sbc100 added a commit to sbc100/emscripten that referenced this pull request Dec 18, 2024
Once we start using `async` for for this function then `new` stops
working: emscripten-core#23157.

Using `new` in this was was always an antipattern but there was nothing
stopping users from doing this.
sbc100 added a commit that referenced this pull request Dec 18, 2024
Once we start using `async` for for this function then `new` stops
working: #23157.

Using `new` in this was was always an antipattern but there was nothing
atopping users from doing this.
@sbc100 sbc100 force-pushed the await_createWasm branch 2 times, most recently from a7bb4fa to 9e0033a Compare December 18, 2024 22:59
@sbc100 sbc100 requested a review from kripken December 18, 2024 23:00
tools/emscripten.py Outdated Show resolved Hide resolved
tools/emscripten.py Outdated Show resolved Hide resolved
ChangeLog.md Outdated Show resolved Hide resolved
@sbc100 sbc100 requested a review from kripken December 19, 2024 00:41
The advantage if using `await` in the cases where we can is that it
avoids the generation or wrapper function for each export.

So instead of

```
var wasmExport = createWasm(); // returns empty object
...
var malloc = ((..) => (malloc = wasmExports['malloc'])()
```

We can just generate:

```
var wasmExport = createWasm(); // returns empty object
...
var malloc = wasmExports['malloc'];
```

This only currently works in MODULARIZE mode where the code is running
inside a factory function.  Otherwise it would end up using
top-level-await.

One wrinkle here is that this is not currently supported when closure is
enabled because we run closure only on the contents of the factory
function so closure ends up seeing this as a top level await when its
not.
ChangeLog.md Outdated Show resolved Hide resolved
@sbc100 sbc100 merged commit b330159 into emscripten-core:main Dec 19, 2024
7 of 14 checks passed
@sbc100 sbc100 deleted the await_createWasm branch December 19, 2024 01:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants