-
Notifications
You must be signed in to change notification settings - Fork 58
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
Sync Methods for FileSystemSyncAccessHandle in File System Access API #772
Comments
I'm wondering, what's the motivation for the sync API? I think async APIs should be friendly to WebAssembly in the near future, given that WebAssembly JavaScript Promise Integration reached Phase 3 in the Wasm CG. |
There's a lot of great discussion by WASM developers on whatwg/fs#7 which illustrate the need for this change. This comment (sampled below) provides a pretty good summary of my thoughts:
|
I am having trouble following this thread. It sounds like the user-facing requirement is that some C++ code can make a blocking call. My understanding is that this is what WebAssembly JS Promise integration provides. These are two ways to implement the same high-level user requirement; it's not the kind of thing user demand will inform one way or the other. Is the concern that JSPI is lower performance? Do you have any benchmarks that show this penalty? I wonder if this benchmark can be used to fix/further optimize JSPI (whether in its core design or various implementations of it), regardless of in what form this feature ships. The reason I am interested in this area is, I worked on the design of JavaScript WeakRefs, where we also had a sync method created for Wasm, and removed it for the reasons @fgmccabe mentions in the thread. I am wondering how we should handle similar cases going forward (as this question can reasonably come up for many async APIs, hence it being a relevant concern for the TAG). |
If the high-level concern here is that we don't want to end up in a world with sync and async versions of every API, then, well, I agree :) This API is a particularly performance-oriented exception, not the rule. The purpose of this API is performance. SyncAccessHandles are intended to provide the most low-level persistent file primitive to the web. This primitive allows sites to bring their own database to the web via Wasm in a way that's orders of magnitude faster than IndexedDB, for example. Also I realize this isn't clear based on the issue description, but SyncAccessHandles are only available from DedicatedWorkers specifically because we don't want to impact the responsiveness of sites. |
This broke our shipped code. We were using an API that landed in a release of chrome. We have a POSIX filesystem built on top of these APIs (we do not have the problems of asyncify, not using it) that we've shipped to customers, we were relying on the APIs that landed in a release of chrome (after a long time of being roped off, mind you) and this change broke our customers. We now have to release an emergency fix. Please, PLEASE, find other ways to do this in the future. |
Hi @hcldan! Really sorry to hear that. We did have a blog post aimed at developers that described the breaking change: https://developer.chrome.com/blog/sync-methods-for-accesshandles/. I recommend you subscribe to the blog’s RSS feed if you haven’t already. Again, sorry this broke your product. If there’s better ways we could have reached developers like you or if there are places you looked at but that didn’t hint at the upcoming breakage, please let me know. Ideally in private, as to not spam this thread with unrelated stuff. Thanks! |
What's the current state here? Given that the main motivation for this is WASM and the WASM people are saying they are working on it from their end (WASM Promises Integration is at Phase 3), is it still necessary to have this feature? |
The fully-synchronous interface* is shipped and in active use on all major browsers. Given the usage, I don't think deprecating this feature if feasible, if that's what you're asking. You bring up a good point, though. Given the progress we're seeing with JSPI would we create the I'm not sure what the performance delta is between JSPI and Asyncify, but presumably the basically-no-overhead approach of That being said, JSPI makes an async alternative to *note that this specific issue only tracks making the entire **until you hit the device driver, at least. Though arguably the decision for browsers to implement the |
Given that this shipped we're closing the review. While we see the use cases, we do have concerns about adding more synchronous APIs where async may have been more appropriate. We hope that once WASM Promises Integration is available that there is some thought given to deprecating these methods. |
Wotcher TAG!
I'm requesting a TAG review of Sync Methods for FileSystemSyncAccessHandle in File System Access API.
FileSystemSyncAccessHandle
is a file primitive that provides performant access to local files. One of its main use cases is applications porting C/C++ code to Wasm; however, asynchronous calls are not fully supported on Wasm yet, and using Asyncify library as an alternative has substantially degraded performance. Also, a split async/sync interface is not ergonomic, as Wasm-based applications expect a synchronous, POSIX-like file API.Therefore, the proposal is to update asynchronous
getSize()
,truncate()
,flush()
andclose()
methods inFileSystemSyncAccessHandle
in File System Access API to synchronous method, in line with synchronousread()
andwrite()
methods.This change can potentially cause breakage if async methods were used the Promise .then()/.catch()/.finally() methods directly. (If used with
await
, there is no breakage). However, the current usage ofFileSystemSyncAccessHandle
is very low (e.g. zero usage queried by HttpArchive, recent shipping on Chrome-only); therefore, the scope and the impact of breakage is expected to be very minimal. Also,FileSystemSyncAccessHandle
was shipped recently on Chrome M102 and is in the process of being implemented in Firefox, so it is preferred to make this change as soon as possible before the API gets used more widely.Further details:
FileSystemSyncAccessHandle contains async methods whatwg/fs#7
Multiple asynchronous operations whatwg/fs#28
You should also know that...
FileSystemSyncAccessHandle
is available in Worker-only, thereby all synchronous calls are invoked from Workers, not from window.FileSystemSyncAccessHandle
, and separately, we plan on supporting an async access handle, which may be more appropriate for common use cases, following asynchronous design patterns.We'd prefer the TAG provide feedback as (please delete all but the desired option):
💬 leave review feedback as a comment in this issue and @-notify [github usernames]
whatwg/fs#7
@dslee414, @a-sully
The text was updated successfully, but these errors were encountered: