-
Notifications
You must be signed in to change notification settings - Fork 22
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
FileSystemSyncAccessHandle contains async methods #7
Comments
This is already being discussed here: WICG/file-system-access#323 People are actually requesting fully sync OPFS surface. |
After chatting with @tomayac and @hannes, I'd also like to summarize a few points here that we find noteworthy regarding the asynchronous methods in the synchronous access handles. We recently entered WebAssembly territory with a browser version of the analytical database DuckDB (available at https://github.com/duckdb/duckdb-wasm). The problem is the following:
I've seen 2 workarounds for this problem which are both insufficient today:
The vision: We think our points here are very similar to the arguments raised for the WebAssembly version of SQLite. |
@fivedots FYI. |
Thank you for the feedback! At this point it's clear to me that the split sync/async interface was a mistake. I'm in agreement that I'd like to see an all-sync* interface. I'm happy to put up a PR once #21 lands. Note that changing methods from async->sync is a web-exposed change with potential breakage, since although This change would affect at least #3 and #28 Chromium implementation bug: https://crbug.com/1338340 *all-sync meaning that all methods on the |
Good. It really never made sense to me that the API specifically to provide sync access was (mostly) async. So... why can't createSyncAccessHandle (aka open()) be sync? This would require spinning event loops to wait for the IPCs needed to complete. However, that's not fundamentally different than what happens if the preallocated quota is exceeded by a write(), the content process will need to request more quota from the main process, and freeze the content in the meantime, since write() is sync. createSyncAccessHandle can only happen on a worker, so if can be frozen/spinning. |
That's a great question, and after doing a bit more digging I realized that some of the assumptions I'd held weren't as solid as I thought they were. The primary argument (and we can debate its strength) is that keeping this async retains some flexibility to put UI, such as permission prompts, on A counter-argument is that by the time you have a file handle to call |
I know webassembly today is quite troublesome to use with async, but I do want to call out that webassembly folks are working hard on a variety of fronts. They're working to make component-model have good async interop (and bonus wasi meeting notes link). There's ideas to further significantly optimize wasm async performance, such as with https://github.com/WebAssembly/js-promise-integration/blob/main/proposals/js-promise-integration/Overview.md. I know relief would not come right away even if this wasm work was all done, that there's still challenges. And I don't have specific use cases for why we'd want to be able to sequence these calls asynchronously. Yet, I feel like the proposal here is to make a less capable, less flexible system, with a more legacy style/feel, to help a target that might not necessarily need to exist for a long time. I'd prefer we wait & see or make a sync alternative, before making a generally less capable system, that hogs threads during not-fast i/o operations. Targeting some specific users ease of use at the cost of general flexibility feels like a violation of Extensible Web Manifesto. As a web developer, having a bunch of sync apis show up that I cannot track & reason about normally seems intimidating, and doesn't feel like a pleasurable or modern experience for webdev. Many of these operations seemingly must block- flush, getSize probably. What are non-wasm webdevelopers expected to do? I don't see Can we talk with some of the other wasm groups & see what we expect the future performance will be? There were a lot of very high visibility folks asking to prioritize wasm, and I do want to give them a no-compromise option. Maybe we should go back to where we were & have both sync & async interfaces available. In general, my inclination is that everything ought be async if at all possible. I'm hoping above all the |
@a-sully: As an outsider to this discussion, I need to take issue with:
The web as a whole, and many other OSes, are 'moving' to an async world. The reason is that that fits real world requirements (responsive apps) much better. Sticking with a sync interface is not compatible with the Web. There are wasm technologies coming that would make an async interface feasible. In particular, I am involved with JS-Promise integration. That is a standards-track activity; which makes it slower. However, our preliminary results seem pretty positive. |
I agree with the overall sentiment here. From my perspective as a Chromium engineer, we are currently focused on making SyncAccessHandles as performant as possible to support C(++) applications being ported to the web via WASM, but we are very interested to hear developer feedback about whether an asynchronous alternative to SyncAccessHandles is worth pursuing. One could imagine an async alternative could be more webby: available from Window contexts etc, using a streams-based interface with built-in queueing, etc. To this point we haven't seen much developer demand for this async alternative. If you have compelling use cases for an async alternative, we'd love to hear your feedback :) I just want to clarify one big point before continuing...
SyncAccessHandles can only be created from a DedicatedWorker (and are intentionally not Okay, so here's the reality of the current landscape:
I'm excitedly following the developments to improve async support in WASM. My hope is that this will allow an async alternative to SyncAccessHandles to have comparable performance characteristics, at which point any web developer looking for fast storage will have no reason to choose the more restrictive SyncAccessHandles. That being said, it's also unclear to me whether C(++) applications being ported to WASM would be able to use or benefit from this async alternative as easily as SyncAccessHandles. There may always be a need for a synchronous file API to support porting these applications to the web. The current split interface is the worst of both worlds. We'll never be able to expose SyncAccessHandles to anything other than DedicatedWorkers due to the existence of sync methods. However, the split sync/async interface forces the application to write effectively sync code anyways, since you can't queue operations due to the sync methods. The result is a significantly slower and more complicated interface than if all the methods were sync. I'd like to see SyncAccessHandles be the MOST useful for WASM-compiled applications, while keeping an ear out for WASM ecosystem async support + developer demand for an async alternative. |
Since making FileSystemSyncAccessHandles fully sync is something of a breaking change, how should we go about it? While a few apps are using this API already (Photoshop), they're still in beta. I imagine they could check if say seek to 0 returns an integer or a promise, which would be pretty easy, and if it's the old promise version wrap it in something like syncseek(...) { return await seek(...); } |
While the changes we're making are technically not backwards-compatible, my hope is that most apps use the async methods of this API with
The big question, in my opinion, is whether we want to make Pro-sync:
Pro-async:
I'm not particularly convinced by the pro-async arguments. I think my preference (and I suspect @jesup will be on board with this, since the non-OPFS portion of the API is not part of this spec) would be to make Thoughts? |
... assuming the WASM performance improvements are as significant as I expect. @tlively can probably speak more to this |
I think for us, the performance of creating a synchronous file handle isn't that big of a deal. That having said, I think that an asynchronous creation of filehandles is something that we could work around ourselves by sacrificing convenience. |
Usually opening this file would require some path traversal first, and that requires directory operations. All directory operations are already async, and I don't think anyone has proposed making them synchronous yet. Given that, opening a file will require working around asynchrony and will be a slow operation no matter whether That being said, the more operations we can make synchronous, the better for performance. But in this case it will be an incremental improvement rather than a panacea. |
Thank you for the input! It's useful to see that WASM developers seem mostly indifferent about whether To summarize, it seems like there's a very strong argument for making all the methods of the SyncAccessHandle sync, but the argument for making At this point, I plan to move forward with my initial proposal (#7 (comment)) of making only the methods of the SyncAccessHandle sync (as soon as #21 lands). If anyone else can provide a strong case to make |
It looks like we're all set for launching this in Chrome 108: |
The spec was updated in #55 so this issue can now be closed. Thank you all for the great discussion and for helping to make the web better! |
Besides
read
andwrite
which are sync methods, there are async methodstruncate
,getSize
,flush
andclose
, returning promises.Does it make sense for the dedicated interface available only in dedicated workers to have async methods ?
The text was updated successfully, but these errors were encountered: