Repository navigation
Support asynchronous context creation #272
Description
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:
| option | async | sync |
|---|---|---|
| async postfix | createContextAsync |
createContext |
| sync postfix | createContext |
createContextSync |
There is a corresponding issue webmachinelearning/model-loader#38 in model loader repo.
/cc @yuhonglin
Activity
huningxin commentedon Jun 10, 2022
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
huningxin commentedon Jun 13, 2022
Label this issue as "cr" blocker because it blocks #229 and #230 which are labeled "cr".
huningxin commentedon Jun 13, 2022
For option "sync postfix", we also need to make the naming of
computeandbuildconsistent.ML.createContextML.createContextSyncMLGraphBuilder.buildMLGraphBuilder.buildSyncMLContext.computeMLContext.computeSyncIn particular, @wacky6 proposed to change the default
buildas async andbuildSyncfor sync version.Introduce MLContext.createContextAsync
domenic commentedon Aug 30, 2022
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
createSyncAccessHandle()method. The returnedSyncAccessHandleinstance has sync methods, instead of the normalAccessHandle's async ones. However, this is generally thought of as regrettable, so try to avoid this path if you can.(Note, however, that
createSyncAccessHandle()itself is async! This is because that method doesn't meet criterion (b); only the actual reads/writes, i.e.syncAccessHandle.read()/write()methods, meet criterion (b).)If you do go the worker-only sync API route, as for naming, there are not many strong precedents here.
createSyncAccessHandle()is one; the old, Chrome-onlyrequestFileSystemSync()and its various pieces is another. (In doing research I also found this 2013 blog post arguing against the idea of sync-only worker APIs!) On the other end, there'sAtomics.waitAsync(). My slight preference would be to distinguish the sync API as the unusual one, so follow thex()+xSync()pattern.anssiko commentedon Aug 30, 2022
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
x()+xSync()naming. Sync APIs are considered an anti-pattern (and as such at risk of deprecation in the future) and thus shall be "earmarked" as unusual (vendor prefixed APIs come to my mind here as an analogy).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
xAsync()+x():createRenderPipelineAsync()/createRenderPipeline()createComputePipelineAsync()/createComputePipeline()mapAsync()/ (no sync counterpart)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.
huningxin commentedon Aug 30, 2022
The sync counterpart
mapSync()is being discussed in gpuweb/gpuweb#2217.wacky6 commentedon Aug 31, 2022
+1 on @domenic's comment.
anssiko commentedon Sep 9, 2022
Per https://www.w3.org/2022/09/08-webmachinelearning-minutes.html#r01 TAG recommendation requested w3ctag/design-reviews#771
zolkis commentedon Oct 19, 2022
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
Syncpostfix (as exception), rather than the Async postfix (which should be the default).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.