-
Notifications
You must be signed in to change notification settings - Fork 21
WASI MemoryFileSystem #230
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
base: main
Are you sure you want to change the base?
Conversation
|
CI failure seems unrelated to these changes (out of storage space,) but if they are let me know. |
| func getPreopenPaths() -> [String] | ||
|
|
||
| /// Opens a directory and returns a WASIDir implementation. | ||
| func openDirectory(at path: String) throws -> any WASIDir |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if the use of existentials is justified here? Could we use opaque types instead?
| func openDirectory(at path: String) throws -> any WASIDir | |
| func openDirectory(at path: String) throws -> some WASIDir |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WASIDir seems to be defined as a protocol so I believe an existential is how its used elsewhere (i.e in Directory.swift)
| import SystemPackage | ||
|
|
||
| /// Base protocol for all file system nodes in memory. | ||
| internal protocol MemFSNode: AnyObject { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a strong reason to require AnyObject here? What would exactly classes (as opposed to non-copyable types) bring to the table other than reference counting performance hit?
| internal protocol MemFSNode: AnyObject { | |
| protocol MemFSNode { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Multiple file entries need to share mutable access to the same node (e.g., a file opened twice should see the same content). With classes, this is natural, both entries hold references to the same object. With non-copyable structs, we'd need an indirection layer (handles/indices into a central store), which adds complexity. Open to that refactor if the ref-counting overhead is measurable or if I am mistaken (Swift is arguably very new to me.)
| /// | ||
| /// This implementation provides access to actual files and directories on the host system, | ||
| /// with appropriate sandboxing through pre-opened directories. | ||
| public final class HostFileSystem: FileSystemProvider, FileSystem { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Usual concern WRT classes overhead here. IIUC we're not relying on class hierarchies, so why not make this a non-copyable struct to avoid the ARC overhead?
| public final class HostFileSystem: FileSystemProvider, FileSystem { | |
| public struct HostFileSystem: FileSystemProvider, FileSystemImplementation, ~Copyable { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason I went with class here is that FileSystemImplementation needs to be stored as a property in WASIBridgeToHost, and Swift's existential types (any Protocol) don't support ~Copyable types. If we make this a non-copyable struct, we'd need to make WASIBridgeToHost generic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kateinoigakukun do you have strong opinions about this? If these were non-public APIs I'd be ok in principle, but wouldn't be great to commit to classes and existentials here without providing high-performance alternatives for those who need it.
|
@andrewmd5 would you mind clarifying what's your use case here? I wonder if a cleaner way to do it is to focus on WASI 0.2+ and Component Model support in WasmKit instead. Then in-memory FS could be implemented in a separate component instead of WasmKit itself. |
|
Running Perl in Swift.
https://github.com/6over3/PerlKit
A memory file system is how it's done for our TypeScript runtime:
https://github.com/6over3/zeroperl-ts
I chatted with Yuta offline and he didn't seem to have any hold ups. Given the lack of real support for WASI 0.2+ I don't think the component model is a realistic solution.
Sent via Superhuman ( ***@***.*** )
…On Sun, Jan 4, 2026 at 6:14 PM, Max Desiatov < ***@***.*** > wrote:
*MaxDesiatov* left a comment (swiftwasm/WasmKit#230) (
#230 (comment) )
@andrewmd5 ( https://github.com/andrewmd5 ) would you mind clarifying
what's your use case here? I wonder if a cleaner way to do it is to focus
on WASI 0.2+ and Component Model support in WasmKit instead. Then
in-memory FS could be implemented in a separate component instead of
WasmKit itself.
—
Reply to this email directly, view it on GitHub (
#230 (comment) ) ,
or unsubscribe (
https://github.com/notifications/unsubscribe-auth/AAJ4VNMGB2CEYDTEFMYI7JL4FDKYHAVCNFSM6AAAAACNHY5VE2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTOMBXHA4TKNZXGE
).
You are receiving this because you were mentioned. Message ID: <swiftwasm/WasmKit/pull/230/c3707895771
@ github. com>
|
Would you elaborate? Lack of support in WasmKit or some other Wasm runtimes? |
|
There is a lack of real support for WASI 0.2+ within the WebAssembly ecosystem. A single runtime supports it - basically no tooling does, including Binaryen. Which is necessary for asyncify. I don't see that changing; the component model offers very little incentive to rewrite code.
Also, if possible, could you share any specific metrics or benchmarks that highlight the performance impact you’re concerned about? The code I provided has benchmarks using this PR and they are great. This isn't exactly a hot path.
Sent via Superhuman ( ***@***.*** )
…On Mon, Jan 5, 2026 at 2:13 AM, Max Desiatov < ***@***.*** > wrote:
*MaxDesiatov* left a comment (swiftwasm/WasmKit#230) (
#230 (comment) )
>
>
> Given the lack of real support for WASI 0.2+ I don't think the component
> model is a realistic solution.
>
>
Would you elaborate? Lack of support in WasmKit or some other Wasm
runtimes?
—
Reply to this email directly, view it on GitHub (
#230 (comment) ) ,
or unsubscribe (
https://github.com/notifications/unsubscribe-auth/AAJ4VNK3O5KW7X34BQY5DOD4FFC3ZAVCNFSM6AAAAACNHY5VE2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTOMBYGI3DANJTGU
).
You are receiving this because you were mentioned. Message ID: <swiftwasm/WasmKit/pull/230/c3708260535
@ github. com>
|
|
I agree with @andrewmd5's point regarding the reality of poor implementation status for Component Model in the ecosystem. So overall, it makes sense to me to add an in-memory FS implementation alongside the native implementation. On the other hand, I think we need to refine the added APIs in this pull to be more Embedded Swift/performance-wise friendly before landing. At least we can avoid most of the existential container usage. If you don't mind, I can push my proposal to this branch. wdyt @andrewmd5 ? |
|
Sounds good to me.
Sent via Superhuman ( ***@***.*** )
…On Tue, Jan 6, 2026 at 10:09 AM, Yuta Saito < ***@***.*** > wrote:
*kateinoigakukun* left a comment (swiftwasm/WasmKit#230) (
#230 (comment) )
I agree with @andrewmd5 ( https://github.com/andrewmd5 ) 's point regarding
the reality of poor implementation status for Component Model in the
ecosystem. So overall, it makes sense to me to add an in-memory FS
implementation alongside the native implementation.
I think we need to refine the added APIs in this pull to be more Embedded
Swift/performance-wise friendly before landing. At least we can avoid most
of the existential container usage. If you don't mind, I can push my
proposal to this branch. wdyt @andrewmd5 ( https://github.com/andrewmd5 ) ?
—
Reply to this email directly, view it on GitHub (
#230 (comment) ) ,
or unsubscribe (
https://github.com/notifications/unsubscribe-auth/AAJ4VNJSBDX342KZCL3GO4T4FMDNDAVCNFSM6AAAAACNHY5VE2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTOMJSGY4DSOBYHA
).
You are receiving this because you were mentioned. Message ID: <swiftwasm/WasmKit/pull/230/c3712689888
@ github. com>
|
|
@andrewmd5 It seems I don't have access to push your branch, so would you mind cherry-picking my change to your branch if you are happy with it? 23dc9f7 |
Adds an in-memory file system to the WASI bridge (with the ability to mount actual file descriptors.)