-
Notifications
You must be signed in to change notification settings - Fork 695
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
Proposal: Memory grow event / handler #1210
Comments
Maybe that's a more common js API: const m = WebAssembly.Memory({ ... });
m.addEventListener("change", fn);
I think that It would be quite confusing for people to have to handle that and doesn't seem like a common use-case in JS. Maybe we could have a live view of the Memory object somehow? |
Sure, a "live view" of sorts that survives a buffer becoming detached would be ideal, if that's an option. Regarding an event: |
malloc can enlarge the memory from within the Wasm code; so in my JS glue on every operation that accesses the memory views I have to check that the view hasn't become detached, and return a fresh view if so. I do this with a wrapper:
You get the idea... a way to avoid this nuisance would be nice! |
Something like this seems useful. It would be useful to collect precedent in other Web APIs, and make this proposal more concrete. |
@xtuc @NWilson Are you providing JS glue for arbitrary wasm modules? If you're generating the modules yourself it seems like it would be better to add an import hook instead. Even if not, you could create a new module with the hook by searching for all @dcodeIO |
@binji That's right, technically this doesn't affect the Effectively it's still the memory that is being grown, with the arraybuffer becoming detached as a side-effect, hence the proposal for adding the event to the memory instance - but I'm of course fine with any more clever way to detect or even avoid the condition in question. |
I think the Observables proposal (in stage 1) is intended to offer an alternative to |
I don't think relying on Observable is a good idea; I don't expect them to be soon in JS (if at all) and that would be very uncommon. The |
Yes, for arbitrary modules. The import hook would be awkward, since the grow_memory instructions are inside libc, which shouldn't really call external functions unless necessary... I guess I could maybe make it a Wasm syscall. There is precedent for Wasm-specific syscalls in Musl. Post-processing with an external tool would be another solution, but it would be a bit of a fiddle (doable I agree). |
Related: #1271, which discusses a mechanism that would make handlers unnecessary. |
Would this event be fired synchronously? So any function that potentially allocates, and therefore potentially grows memory, could potentially run JS when it didn't before? |
In #1271 (comment), it's pointed out that a synchronous event can't work anyway because a different thread can grow the memory. An asynchronous event seems error-prone: a webassembly function that merely calls malloc or new before calling an imported javascript function that references the webassembly's memory buffer will occasionally fail because malloc/new will sometimes cause the memory to grow, and if the webassembly doesn't release control of the event loop between those steps, there's no opportunity for the asynchronous event to be dispatched and for the javascript to update its buffer references and views. (A grow event might be useful for other things, but I don't think it is for solving the buffer/view-updating problem.) |
I have a use case here where a WebAssembly module is able to grow its memory from the inside, making it necessary to import a function from JS that is called to notify the outside that the underlying buffer is now in a detached state and that any potential views on the memory buffer must be recreated.
It would be easier / more interoperable / involve less boilerplate if the
WebAssembly.Memory
instance would emit an event / call a handler whenever growing of memory happens, for example agrow
event / an.ongrow
property that can be assigned a callback.Is this something worth considering? :)
The text was updated successfully, but these errors were encountered: