Skip to content

Conversation

@andrewmd5
Copy link

Adds an in-memory file system to the WASI bridge (with the ability to mount actual file descriptors.)

@andrewmd5
Copy link
Author

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
Copy link
Member

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?

Suggested change
func openDirectory(at path: String) throws -> any WASIDir
func openDirectory(at path: String) throws -> some WASIDir

Copy link
Author

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 {
Copy link
Member

@MaxDesiatov MaxDesiatov Nov 27, 2025

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?

Suggested change
internal protocol MemFSNode: AnyObject {
protocol MemFSNode {

Copy link
Author

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 {
Copy link
Member

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?

Suggested change
public final class HostFileSystem: FileSystemProvider, FileSystem {
public struct HostFileSystem: FileSystemProvider, FileSystemImplementation, ~Copyable {

Copy link
Author

@andrewmd5 andrewmd5 Nov 28, 2025

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.

Copy link
Member

@MaxDesiatov MaxDesiatov Jan 2, 2026

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.

@MaxDesiatov
Copy link
Member

MaxDesiatov commented Jan 4, 2026

@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.

@andrewmd5
Copy link
Author

andrewmd5 commented Jan 4, 2026 via email

@MaxDesiatov
Copy link
Member

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?

@andrewmd5
Copy link
Author

andrewmd5 commented Jan 4, 2026 via email

@kateinoigakukun
Copy link
Member

kateinoigakukun commented Jan 6, 2026

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 ?

@andrewmd5
Copy link
Author

andrewmd5 commented Jan 6, 2026 via email

@kateinoigakukun
Copy link
Member

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants