Add a new way to mark async imports in library files. #19093
Conversation
brendandahl
force-pushed
the
async-imports
branch
from
66c66e5 to
a0e8a5a
Compare
src/jsifier.js
Outdated
| @@ -423,6 +430,9 @@ function ${name}(${args}) { | |||
| if (sig && (RELOCATABLE || ASYNCIFY == 2)) { | |||
| contentText += `\n${mangled}.sig = '${sig}';`; | |||
| } | |||
| if (isAsyncFunction) { | |||
There was a problem hiding this comment.
Should we only output these .isAsync properties when ASYNCIFY is enabled?
Can isAsyncFunction ever be true when ASYNCIFY is not set? i.e. what does it mean to have AsyncFunction or __async attribute when ASYNCIFY is not enabled? Should we assert here maybe?
There was a problem hiding this comment.
I changed it to only output when ASYNCIFY is enabled.
Can isAsyncFunction ever be true when ASYNCIFY is not set? i.e. what does it mean to have AsyncFunction or __async attribute when ASYNCIFY is not enabled? Should we assert here maybe?
Currently it won't ever be true, but yeah it should probably error if you're not using asyncify with async stuff. However, there may be some users that use async fn() in their libraries without asyncify. I'm thinking I'll just add an assert for $symbol__async.
brendandahl
force-pushed
the
async-imports
branch
from
a0e8a5a to
17c306a
Compare
| else: | ||
| lines.append(name) | ||
| write_file(filename, '\n'.join(lines) + '\n') | ||
| write_file(filename, json.dumps(library_syms, separators=(',', ':'))) |
There was a problem hiding this comment.
This isn't quite as compact as the previous format, but JSON should be fast and it seems easier than maintaining our own serialize/deserialize now that there's more meta data.
emcc.py
Outdated
| else: | ||
| lines.append(name) | ||
| write_file(filename, '\n'.join(lines) + '\n') | ||
| write_file(filename, json.dumps(library_syms, separators=(',', ':'))) | ||
|
|
||
| # We need to use a separate lock here for symbol lists because, unlike with system libraries, | ||
| # it's normally for these file to get pruned as part of normal operation. This means that it | ||
| # can be deleted between the `cache.get()` then the `read_file`. | ||
| with filelock.FileLock(cache.get_path(cache.get_path('symbol_lists.lock'))): | ||
| filename = cache.get(f'symbol_lists/{content_hash}.txt', build_symbol_list) |
There was a problem hiding this comment.
Maybe replace .txt here with .json if that is what it is now.
brendandahl
force-pushed
the
async-imports
branch
from
d03f06e to
e1f9637
Compare
Allow library functions to be asyncify'd with either an `async` function or adding `$name_async`.
brendandahl
force-pushed
the
async-imports
branch
from
e1f9637 to
1849d5e
Compare
In #19093 both async functions and functions marked with `NAME__async: true` were marked as functions to be processed by Asyncify. However, looking at usages, standalone async functions are currently used with emscripten_proxy_finish-based callbacks and not with Asyncify. Besides, such functions can't work with Asyncify unless we add automatic `Asyncify.handleAsync` wrapping around each such function as well. We could do that, but given that there are already is some confusion around what async means for any particular function, it seems better to keep explicit calls to `emscripten_proxy_finish` and `Asyncify.handleAsync` correspondingly. Hence, in this PR I'm not adding more automatic wrapping but instead just removing the assumption for standalone async functions which should somewhat reduce code size as they will no longer be Asyncify-ed when not necessary.
…ync.c that tests this behavior. (does not quite yet work, due to emscripten-core/emscripten#20490) Also remove use of ASYNCIFY_IMPORTS since after emscripten-core/emscripten#19093, that directive is no longer necessary. (see also emscripten-core/emscripten#15746) One important gotcha is that when doing synchronous buffer mapping during rendering time (in a rAF callback), one must then gate further rAFs to be skipped until the synchronous buffer mapping resolves.
Allow library functions to be asyncify'd with either an
asyncfunction or adding$name_async.This is a minimal example, I filed #19090 as a follow up to move more to use this style.