-
Notifications
You must be signed in to change notification settings - Fork 48
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
Support asynchronous context creation #272
Comments
The related discussion in Chromium CL review: https://chromium-review.googlesource.com/c/chromium/src/+/3684745/7/third_party/blink/renderer/modules/ml/ml.idl#16 |
For option "sync postfix", we also need to make the naming of
In particular, @wacky6 proposed to change the default |
This PR also restricts the sync version of create context to worker. Fix webmachinelearning#272
I was asked to chime in here with some background about how other web platform APIs have approached this. In general, the preferred approach is to not have sync APIs like this at all. They are an antipattern on the web platform, and one we've moved away from. This includes in a worker-only context; creating worker-only sync APIs bifurcates the platform and makes it easy for developers to write libraries which are not available in some contexts. It also blocks the worker thread, which can cause its own problems in some cases. This does make using such APIs more work from WebAssembly. There is some hope that in the future this will get easier via the JS promise integration proposal. In the meantime, WebAssembly developers are encouraged to use technologies such as asyncify. Asyncify does have a performance cost, however. If your API (a) really needs to ship before the JS promise integration proposal; and (b) is used repeatedly in a tight loop in core application logic, so that the performance cost of asyncify will basically make the API unusable: then, you might be able to go for an exception, and ship a worker-only sync API. This was done recently for the File System Access Handle API, which has a (Note, however, that If you do go the worker-only sync API route, as for naming, there are not many strong precedents here. |
Thank you @domenic. This new background information is very helpful for the WG and was delivered just in time. Those of you who don't yet know @domenic: he wears many hats such as ex-TAG and TC39 to name a few and would be my go-to person for this type of questions. Reading @domenic's comment and reflecting on it, I'm hearing a slight preference to In the (perfect) future we'd have all APIs async and thus there's no need to emphasize this default in naming, and should strive for conciseness in naming instead. This design is staged in PR #274. The another approach discussed in the WG is to align with the WebGU API's emerging naming convention
This design is staged in PR #285. To close, let me quote Joshua Bloch just in case this provides some food for thought to someone watching:
All - let's continue this design discussion in this issue instead of the two PRs to not fork our discussion. |
The sync counterpart |
+1 on @domenic's comment. |
Per https://www.w3.org/2022/09/08-webmachinelearning-minutes.html#r01 TAG recommendation requested w3ctag/design-reviews#771 |
I am scanning issues on topics of current interest (among which, in this case, context), and found this. I was about to ask why don't we use async API by default, and check whether resolving a promise immediately is good enough approximation for sync use cases. Then I've read Domenic's comment and realized WASM is the reason, and it's a valid use case to be able to specify fully synchronous behavior. Anyway, since APIs stay, a good practice IMHO would be to use async API wherever possible, and postfix the exceptions, i.e. use the As I studied the API structure, I was wondering if we can do some simplification about context creation, since the algorithms (not necessarily the implementation) would become simpler, and might solve this, too. I create a separate issue. |
The context creation might be time consuming. For example, the implementation (e.g., Chromium) may use inter-process-communication (IPC) to query hardware capabilities and allocate hardware resources in another process. The synchronous call of current
ml.createContext
may block the main thread and impact the responsiveness of the user interface.The proposal is to introduce an async version of
createContext
and restrict the sync version to be only used by worker thread. This also aligns with the discussions of #229 and #230.The model loader API has an async version of
createContext
. It would be better to have a consistent naming for sync and async methods between the two WebML APIs. There are two options:createContextAsync
createContext
createContext
createContextSync
There is a corresponding issue webmachinelearning/model-loader#38 in model loader repo.
/cc @yuhonglin
The text was updated successfully, but these errors were encountered: