Skip to content

Commit 970998b

Browse files
tlivelytlively
authored
[WasmFS][NFC] Simplify the async JS backend code (#16894)
Remove the CallbackState argument in favor of directly passing outparam pointers for results and remove the callback function in favor of passing the em_proxying_ctx and directly calling emscripten_proxy_finish on it. Also remove the unnecessary `runtimeKeepalivePush` and `runtimeKeepalivePop` calls since the `ProxyWorker` utility already takes care of keeping the runtime alive.
1 parent c78fddf commit 970998b

File tree

7 files changed

+46
-152
lines changed

7 files changed

+46
-152
lines changed

‎emcc.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1531,6 +1531,10 @@ def setup_pthreads(target):
15311531
if settings.PROXY_TO_PTHREAD:
15321532
settings.REQUIRED_EXPORTS += ['emscripten_proxy_main']
15331533

1534+
# All proxying async backends will need this.
1535+
if settings.WASMFS:
1536+
settings.REQUIRED_EXPORTS += ['emscripten_proxy_finish']
1537+
15341538
# pthread stack setup and other necessary utilities
15351539
def include_and_export(name):
15361540
settings.DEFAULT_LIBRARY_FUNCS_TO_INCLUDE += ['$' + name]

‎src/library_wasmfs_jsimpl.js

Lines changed: 15 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -60,65 +60,52 @@ mergeInto(LibraryManager.library, {
6060
// WASM_BIGINT so the pointer is just a single argument, just like in
6161
// wasm32).
6262
_wasmfs_jsimpl_async_alloc_file__deps: ['$runtimeKeepalivePush', '$runtimeKeepalivePop'],
63-
_wasmfs_jsimpl_async_alloc_file: async function(backend, file, fptr, arg) {
63+
_wasmfs_jsimpl_async_alloc_file: async function(ctx, backend, file) {
6464
#if ASSERTIONS
6565
assert(wasmFS$backends[backend]);
6666
#endif
67-
{{{ runtimeKeepalivePush() }}}
6867
await wasmFS$backends[backend].allocFile(file);
69-
{{{ runtimeKeepalivePop() }}}
70-
{{{ makeDynCall('vi', 'fptr') }}}(arg);
68+
_emscripten_proxy_finish(ctx);
7169
},
7270

7371
_wasmfs_jsimpl_async_free_file__deps: ['$runtimeKeepalivePush', '$runtimeKeepalivePop'],
74-
_wasmfs_jsimpl_async_free_file: async function(backend, file, fptr, arg) {
72+
_wasmfs_jsimpl_async_free_file: async function(ctx, backend, file) {
7573
#if ASSERTIONS
7674
assert(wasmFS$backends[backend]);
7775
#endif
78-
{{{ runtimeKeepalivePush() }}}
7976
await wasmFS$backends[backend].freeFile(file);
80-
{{{ runtimeKeepalivePop() }}}
81-
{{{ makeDynCall('vi', 'fptr') }}}(arg);
77+
_emscripten_proxy_finish(ctx);
8278
},
8379

8480
_wasmfs_jsimpl_async_write__deps: ['$runtimeKeepalivePush', '$runtimeKeepalivePop'],
85-
_wasmfs_jsimpl_async_write: async function(backend, file, buffer, length, {{{ defineI64Param('offset') }}}, fptr, arg) {
81+
_wasmfs_jsimpl_async_write: async function(ctx, backend, file, buffer, length, {{{ defineI64Param('offset') }}}, result_p) {
8682
{{{ receiveI64ParamAsDouble('offset') }}}
8783
#if ASSERTIONS
8884
assert(wasmFS$backends[backend]);
8985
#endif
90-
{{{ runtimeKeepalivePush() }}}
91-
var size = await wasmFS$backends[backend].write(file, buffer, length, offset);
92-
{{{ runtimeKeepalivePop() }}}
93-
{{{ makeSetValue('arg', C_STRUCTS.CallbackState.result, '0', 'i32') }}};
94-
{{{ makeSetValue('arg', C_STRUCTS.CallbackState.offset, 'size', 'i64') }}};
95-
{{{ makeDynCall('vi', 'fptr') }}}(arg);
86+
var result = await wasmFS$backends[backend].write(file, buffer, length, offset);
87+
{{{ makeSetValue('result_p', 0, 'result', 'i64') }}};
88+
_emscripten_proxy_finish(ctx);
9689
},
9790

9891
_wasmfs_jsimpl_async_read__deps: ['$runtimeKeepalivePush', '$runtimeKeepalivePop'],
99-
_wasmfs_jsimpl_async_read: async function(backend, file, buffer, length, {{{ defineI64Param('offset') }}}, fptr, arg) {
92+
_wasmfs_jsimpl_async_read: async function(ctx, backend, file, buffer, length, {{{ defineI64Param('offset') }}}, result_p) {
10093
{{{ receiveI64ParamAsDouble('offset') }}}
10194
#if ASSERTIONS
10295
assert(wasmFS$backends[backend]);
10396
#endif
104-
{{{ runtimeKeepalivePush() }}}
105-
var size = await wasmFS$backends[backend].read(file, buffer, length, offset);
106-
{{{ runtimeKeepalivePop() }}}
107-
{{{ makeSetValue('arg', C_STRUCTS.CallbackState.result, '0', 'i32') }}};
108-
{{{ makeSetValue('arg', C_STRUCTS.CallbackState.offset, 'size', 'i64') }}};
109-
{{{ makeDynCall('vi', 'fptr') }}}(arg);
97+
var result = await wasmFS$backends[backend].read(file, buffer, length, offset);
98+
{{{ makeSetValue('result_p', 0, 'result', 'i32') }}};
99+
_emscripten_proxy_finish(ctx);
110100
},
111101

112102
_wasmfs_jsimpl_async_get_size__deps: ['$runtimeKeepalivePush', '$runtimeKeepalivePop'],
113-
_wasmfs_jsimpl_async_get_size: async function(backend, file, fptr, arg) {
103+
_wasmfs_jsimpl_async_get_size: async function(ctx, backend, file, size_p) {
114104
#if ASSERTIONS
115105
assert(wasmFS$backends[backend]);
116106
#endif
117-
{{{ runtimeKeepalivePush() }}}
118107
var size = await wasmFS$backends[backend].getSize(file);
119-
{{{ runtimeKeepalivePop() }}}
120-
{{{ makeSetValue('arg', C_STRUCTS.CallbackState.result, '0', 'i32') }}};
121-
{{{ makeSetValue('arg', C_STRUCTS.CallbackState.offset, 'size', 'i64') }}};
122-
{{{ makeDynCall('vi', 'fptr') }}}(arg);
108+
{{{ makeSetValue('size_p', 0, 'size', 'i64') }}};
109+
_emscripten_proxy_finish(ctx);
123110
},
124111
});

‎src/struct_info_cxx.json

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,5 @@
2323
"wasmfs::File::DirectoryKind",
2424
"wasmfs::File::SymlinkKind"
2525
]
26-
},
27-
{
28-
"file": "async_callback.h",
29-
"structs": {
30-
"CallbackState": [
31-
"result",
32-
"offset"
33-
]
34-
}
3526
}
3627
]

‎system/lib/wasmfs/async_callback.h

Lines changed: 0 additions & 27 deletions
This file was deleted.

‎system/lib/wasmfs/proxied_async_js_impl_backend.h

Lines changed: 27 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99

1010
#pragma once
1111

12-
#include "async_callback.h"
1312
#include "backend.h"
1413
#include "memory_backend.h"
1514
#include "support.h"
@@ -41,46 +40,33 @@
4140

4241
using js_index_t = uint32_t;
4342

44-
// A callback state that also adds the C++ component for resuming. This is only
45-
// needed on the C++ side, but needs to be passed around as part of this
46-
// struct when we go through C and JS.
47-
struct CppCallbackState : public CallbackState {
48-
// The function to call to resume execution.
49-
emscripten::ProxyingQueue::ProxyingCtx ctx;
50-
};
51-
5243
extern "C" {
53-
// JSImpl async API.
5444

55-
// An async callback with no extra parameters.
56-
typedef void (*async_callback_t)(CppCallbackState* state);
57-
58-
void _wasmfs_jsimpl_async_alloc_file(js_index_t backend,
59-
js_index_t index,
60-
async_callback_t callback,
61-
CppCallbackState* state);
62-
void _wasmfs_jsimpl_async_free_file(js_index_t backend,
63-
js_index_t index,
64-
async_callback_t callback,
65-
CppCallbackState* state);
66-
void _wasmfs_jsimpl_async_write(js_index_t backend,
45+
// JSImpl async API.
46+
void _wasmfs_jsimpl_async_alloc_file(em_proxying_ctx* ctx,
47+
js_index_t backend,
48+
js_index_t index);
49+
void _wasmfs_jsimpl_async_free_file(em_proxying_ctx* ctx,
50+
js_index_t backend,
51+
js_index_t index);
52+
void _wasmfs_jsimpl_async_write(em_proxying_ctx* ctx,
53+
js_index_t backend,
6754
js_index_t index,
6855
const uint8_t* buffer,
6956
size_t length,
7057
off_t offset,
71-
async_callback_t callback,
72-
CppCallbackState* state);
73-
void _wasmfs_jsimpl_async_read(js_index_t backend,
58+
__wasi_errno_t* result);
59+
void _wasmfs_jsimpl_async_read(em_proxying_ctx* ctx,
60+
js_index_t backend,
7461
js_index_t index,
7562
const uint8_t* buffer,
7663
size_t length,
7764
off_t offset,
78-
async_callback_t callback,
79-
CppCallbackState* state);
80-
void _wasmfs_jsimpl_async_get_size(js_index_t backend,
65+
__wasi_errno_t* result);
66+
void _wasmfs_jsimpl_async_get_size(em_proxying_ctx* ctx,
67+
js_index_t backend,
8168
js_index_t index,
82-
async_callback_t callback,
83-
CppCallbackState* state);
69+
size_t* result);
8470
}
8571

8672
namespace wasmfs {
@@ -103,63 +89,38 @@ class ProxiedAsyncJSImplFile : public DataFile {
10389
void close() override {}
10490

10591
__wasi_errno_t write(const uint8_t* buf, size_t len, off_t offset) override {
106-
CppCallbackState state;
107-
92+
__wasi_errno_t result;
10893
proxy([&](auto ctx) {
109-
state.ctx = ctx;
110-
11194
_wasmfs_jsimpl_async_write(
112-
getBackendIndex(),
113-
getFileIndex(),
114-
buf,
115-
len,
116-
offset,
117-
[](CppCallbackState* state) { state->ctx.finish(); },
118-
&state);
95+
ctx.ctx, getBackendIndex(), getFileIndex(), buf, len, offset, &result);
11996
});
12097

121-
return state.result;
98+
return result;
12299
}
123100

124101
__wasi_errno_t read(uint8_t* buf, size_t len, off_t offset) override {
125102
// The caller should have already checked that the offset + len does
126103
// not exceed the file's size.
127104
assert(offset + len <= getSize());
128105

129-
CppCallbackState state;
130-
106+
__wasi_errno_t result;
131107
proxy([&](auto ctx) {
132-
state.ctx = ctx;
133-
134108
_wasmfs_jsimpl_async_read(
135-
getBackendIndex(),
136-
getFileIndex(),
137-
buf,
138-
len,
139-
offset,
140-
[](CppCallbackState* state) { state->ctx.finish(); },
141-
&state);
109+
ctx.ctx, getBackendIndex(), getFileIndex(), buf, len, offset, &result);
142110
});
143111

144-
return state.result;
112+
return result;
145113
}
146114

147115
void flush() override {}
148116

149117
size_t getSize() override {
150-
CppCallbackState state;
151-
118+
size_t result;
152119
proxy([&](auto ctx) {
153-
state.ctx = ctx;
154-
155120
_wasmfs_jsimpl_async_get_size(
156-
getBackendIndex(),
157-
getFileIndex(),
158-
[](CppCallbackState* state) { state->ctx.finish(); },
159-
&state);
121+
ctx.ctx, getBackendIndex(), getFileIndex(), &result);
160122
});
161-
162-
return state.offset;
123+
return result;
163124
}
164125

165126
void setSize(size_t size) override {
@@ -171,30 +132,16 @@ class ProxiedAsyncJSImplFile : public DataFile {
171132
backend_t backend,
172133
emscripten::ProxyWorker& proxy)
173134
: DataFile(mode, backend), proxy(proxy) {
174-
CppCallbackState state;
175-
176135
proxy([&](auto ctx) {
177-
state.ctx = ctx;
178-
179136
_wasmfs_jsimpl_async_alloc_file(
180-
getBackendIndex(),
181-
getFileIndex(),
182-
[](CppCallbackState* state) { state->ctx.finish(); },
183-
&state);
137+
ctx.ctx, getBackendIndex(), getFileIndex());
184138
});
185139
}
186140

187141
~ProxiedAsyncJSImplFile() {
188-
CppCallbackState state;
189-
190142
proxy([&](auto ctx) {
191-
state.ctx = ctx;
192-
193143
_wasmfs_jsimpl_async_free_file(
194-
getBackendIndex(),
195-
getFileIndex(),
196-
[](CppCallbackState* state) { state->ctx.finish(); },
197-
&state);
144+
ctx.ctx, getBackendIndex(), getFileIndex());
198145
});
199146
}
200147
};

‎tests/reference_struct_info.json

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -375,11 +375,6 @@
375375
"__WASI_FILETYPE_SYMBOLIC_LINK": 7
376376
},
377377
"structs": {
378-
"CallbackState": {
379-
"__size__": 16,
380-
"offset": 8,
381-
"result": 0
382-
},
383378
"EmscriptenBatteryEvent": {
384379
"__size__": 32,
385380
"charging": 24,

‎tools/deps_info.py

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,4 @@ def get_deps_info():
219219
if settings.USE_PTHREADS:
220220
_deps_info['emscripten_set_canvas_element_size_calling_thread'] = ['emscripten_dispatch_to_thread_']
221221

222-
if settings.WASMFS:
223-
_deps_info['wasmfs_create_opfs_backend'] = ['emscripten_proxy_finish']
224-
225222
return _deps_info

0 commit comments

Comments
 (0)