Draft: Add support for read/seek on files in the native file system #16307
Conversation
| } | ||
| }; | ||
| } | ||
| #endif |
There was a problem hiding this comment.
Rather than putting this in the preample, I think you can just create an "NativeFS.init()" function in your library and then use the __postset feature of the library to call that init function on startup.
| // This option depends on the previous one, it controls the size of allocated | ||
| // nativefs structure that contains state information on the files currently open. | ||
| // default value is 1024, maximum number of file descriptors allowed. | ||
| var NATIVEFS_MAX_FD=1024 |
There was a problem hiding this comment.
I seems like you are creating an entirely new FS space.. that sits alongside the existing one. Why not integrate with it such that certain FSs can be from native FS and others from other filesystems? Then you don't need a new API at all and you can just use the normal C fread/fwrite/fopen APIs.
As it stands it seem confusing that fd 1 will exist both as regular fd and a nativefs fd.
| @@ -190,6 +190,9 @@ function callMain(args) { | |||
| assert(ret == 0, '_emscripten_proxy_main failed to start proxy thread: ' + ret); | |||
| #endif | |||
| #else | |||
| #if EMSCRIPTEN_NATIVE_FS | |||
| _nativefs_cleanup(); | |||
There was a problem hiding this comment.
Any, I think it would be good to keep this kind of thing in the library file for nativefs. You can use addAtExit like library_fs.js does.
| "defines": [ | ||
| "NATIVEFS_SEEK_SET", | ||
| "NATIVEFS_SEEK_CUR", | ||
| "NATIVEFS_SEEK_END" |
There was a problem hiding this comment.
Why use different constants to the existing SEEK_XX ones?
| } | ||
| #endif | ||
|
|
||
| #endif NATIVEFS_H |
There was a problem hiding this comment.
NATIVEFS_H should be in a comment here
| var fd = 2; | ||
| var sharedMemory; | ||
| var fs_lock = -1; | ||
| var nativefs_handle; |
There was a problem hiding this comment.
I don't understand why this code has to be in worker.
Why can't the files be stored on the main thread? It looks like its just an array of blobs which should work just as well on the main thread.
There was a problem hiding this comment.
Thank you very much for the review !
I thought it would be easier to have this version first. With Atomics.wait() impossible in the main thread, and having to do async reads instead of sync reads.
I agree it would be better to avoid having to send the file anywhere at all. I will send another version taking your comments into account.
|
I haven't read all the details here, but there is a lot of code. Can you perhaps write up a short overview of what this does, and how? As a general thought: We are working hard on WasmFS now, which will eventually replace the old FS code, so new file-related things should use WasmFS in most cases. Here, could we add a file backend for WasmFS that supports the file picker? (Also, this might be easier to do in WasmFS as it has native support for multithreading.) WasmFS is not quite stable, but implementing some backends is already possible. See #16209 and #16229 for generic mechanisms for interacting with JS. |
It allows a user to compile a multi-threaded C/C++ application that will read/seek files selected in the native filesystem with the file picker. Because I though making calls in the main thread was not ideal, there is a dedicated runtime worker that will receive all the files along with operations to read/seek/cleanup the state. for example: https://github.com/msabwat/emscripten/blob/nfs/2/tests/browser/nativefs/nativefs_seek.c
Thank you for the explanation, it makes more sense to add a wasmfs backend |
|
@msabwat Thanks for the explanation! Given the dedicated worker approach here, I think in WasmFS you can use an async proxied JS backend (#16229). It's pretty new, but what you're doing sounds like a great use case for that. If you don't mind changing APIs atm then you might be interested to try it out. |
Hi,
This is a set of changes to try to solve the issue described in #14094.
This PR will add :
I had an issue when I wanted to pass parameters from pthreads to the main thread, directly from a javascript library. I patched mainThreadEM_ASM to be able to make the calls, without having to define functions in C only to add Macros to inline javascript code.