Skip to content
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

Possibility of sync version of createSyncAccessHandle() #100

Closed
tomayac opened this issue Mar 8, 2023 · 6 comments
Closed

Possibility of sync version of createSyncAccessHandle() #100

tomayac opened this issue Mar 8, 2023 · 6 comments
Labels
addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest

Comments

@tomayac
Copy link
Contributor

tomayac commented Mar 8, 2023

The current createSyncAccessHandle() method is asynchronous. Since we don’t universally have JSPI on all platforms/browsers yet, obtaining a FileSystemSyncAccessHandle can be unergonomic from a Wasm context. We hear developer demand for a synchronous version of createSyncAccessHandle() (obviously tied to Worker contexts). Is this feasible?

@annevk annevk added addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest labels Mar 8, 2023
@a-sully
Copy link
Collaborator

a-sully commented Mar 9, 2023

This was discussed quite a bit on #7 (the issue to make all the methods on the SyncAccessHandle sync). We came to the following conclusion (from #7 (comment)):

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 createSyncAccessHandle sync is less clear-cut, especially considering that applications typically have to deal with async methods when opening a file anyways. To me, the slight performance improvement is not worth the greater risk of site breakage and the sacrifice of future flexibility.
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 createSyncAccessHandle sync and would like to drive that work, then of course they are welcome to :)

@tomayac
Copy link
Contributor Author

tomayac commented Mar 9, 2023

Right. I have created this new Issue because…

If anyone else can provide a strong case to make createSyncAccessHandle sync and would like to drive that work, then of course they are welcome to

…we may have found this “anyone”. I hope for them to chime in on the Issue.

@d3lm
Copy link

d3lm commented Mar 9, 2023

Hello 👋 I am the Team Lead at StackBlitz for WebContainer, a Node.js runtime for the browser. For our runtime we would highly benefit from createSyncAccessHandle being synchronous because our idea is to use the OPFS as another layer in our kernel to allow swapping files out of memory onto disk which can drastically reduce the memory footprint for large applications. Our kernel is compiled to Wasm and a lot of APIs are expected to be synchronous. As Thomas said, without universal support for JSPI that allows to suspend the execution of the Wasm module when calling a function that returns a Promise, the only way we could open files from our kernel is to use a separate thread that performs this operation but that also means we'd have to write to that file on a separate thread (worker) IIUC (similar to how WORKERFS works for emscripten). In an ideal world, we could synchronously open files directly from our kernel, and start swapping the content.

Does that make sense?

@a-sully
Copy link
Collaborator

a-sully commented Mar 10, 2023

Hi @d3lm, thanks for contributing your use case! Just to make sure I understand correctly - you still would need to call the async getFileHandle() method (and maybe also getDirectoryHandle()) to get the FileSystemFileHandle on which you would then call createSyncAccessHandle(), correct?

On that earlier spec issue it was concluded that making createSyncAccessHandle() sync would not provide much benefit, in part because the async directory traversal methods that we never intend to make sync (i.e. getFileHandle() and getDirectoryHandle()) were assumed to be a prerequisite to open the file anyways. If you need to call these async methods anyways, then a sync createSyncAccessHandle() doesn't really help you much. This is the case for Emscripten, for example, which is general-purpose and performs this directory traversal on-demand.

The most convincing use case (courtesy of rhashimoto/wa-sqlite#67) I've seen so far for createSyncAccessHandle() to be sync is if there's a known set of files you plan to interact with. In this case, you could (asynchronously) get all the FileSystemFileHandles you need up front, then read/write/etc all synchronously... at which point an async "open" method is a real hindrance

@d3lm
Copy link

d3lm commented Feb 16, 2024

@a-sully Yes, that is absolutely right. I'd still need to call getDirectoryHandle or getFileHandle. And if there won't be sync variants of those then yea it doesn't make much sense to make createSyncAccessHandle synchronous. I think for our use case we'd need basically the entire chain of API calls to be synchronous so we could use OPFS as a backing store for our file system.

@a-sully
Copy link
Collaborator

a-sully commented Feb 16, 2024

I think for our use case we'd need basically the entire chain of API calls to be synchronous

Yup, this is what I had suspected :)

JSPI is currently in Dev Trial and is going into Origin Trial in M123 in Chrome. I would recommend checking it out!

@a-sully a-sully closed this as completed Feb 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest
Development

No branches or pull requests

4 participants