Skip to content
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

Heap growth notifications #12336

Open
kg opened this issue Sep 25, 2020 · 8 comments
Open

Heap growth notifications #12336

kg opened this issue Sep 25, 2020 · 8 comments

Comments

@kg
Copy link

kg commented Sep 25, 2020

It would be great if updateGlobalBufferAndViews could notify embedder(s) and application code that the heap has been resized, I don't see any way to get at this information currently. Right now if the heap grows, any live typed arrays pointing at the wasm heap become useless garbage, which makes it very difficult to make use of typed arrays safely when interacting with a compiled wasm binary. The fact that detached arrays are intentionally designed to fail silently makes this hard to debug as well. If there were a notification or signal we could process, that would make it possible to at least build a "safe typedarray" abstraction around this with adequate performance characteristics (wrap a typedarray instance, indirect into it with get/set methods, etc)

@kripken
Copy link
Member

kripken commented Sep 29, 2020

I agree this can be annoying, yeah, Web APIs make it easy to miss this and get bad results.

One way to do this right now is to monkeypatch wasmMemory.grow, which is what we call from JS to grow, to run some code in the middle. Another way is to reimplement $emscripten_realloc_buffer in a JS library, which would override the default implementation from library.js (you'd need to do the same things the current code does, but it's just a few lines).

Optimally the wasm JS API would provide a callback, but there isn't agreement on that, WebAssembly/design#1296 😢

@stale
Copy link

stale bot commented Apr 17, 2022

This issue has been automatically marked as stale because there has been no activity in the past year. It will be closed automatically if no further activity occurs in the next 30 days. Feel free to re-open at any time if this issue is still relevant.

@stale stale bot added the wontfix label Apr 17, 2022
@sbc100
Copy link
Collaborator

sbc100 commented Apr 19, 2022

Its possible to wrap library functions do you can re-implement $emscripten_realloc_buffer while calling back to the original. See https://github.com/emscripten-core/emscripten/blob/main/tests/test_override_system_js_lib_symbol.js.

Hopefully that is sufficient for you purposes? Our side of that we could potentially add an opt-in INCOMING_MODULE_JS_API element here? Do you think that second step is needed? If not, I think this issue can be closed with WebAssembly/design#1296 being the long term fix outside of emscripten.

@stale stale bot removed the wontfix label Apr 19, 2022
@kg
Copy link
Author

kg commented Apr 19, 2022

Its possible to wrap library functions do you can re-implement $emscripten_realloc_buffer while calling back to the original. See https://github.com/emscripten-core/emscripten/blob/main/tests/test_override_system_js_lib_symbol.js.

Hopefully that is sufficient for you purposes? Our side of that we could potentially add an opt-in INCOMING_MODULE_JS_API element here? Do you think that second step is needed? If not, I think this issue can be closed with WebAssembly/design#1296 being the long term fix outside of emscripten.

That's what we ended up settling on. It's kind of gross but if we can be sure it'll keep working that's probably fine - reimplementing private (private-looking) symbols starting with $emscripten is mildly scary.

@kg
Copy link
Author

kg commented Apr 19, 2022

Actually, checking again I should note that we're replacing updateGlobalBufferAndViews instead. Is that the wrong place to put our code?

@sbc100
Copy link
Collaborator

sbc100 commented Apr 19, 2022

Either way seems like monkey patching so I would say they are probably equally good, and equally fragile. Still it seems fine, and probably not worth adding an official API hook.

I'm curious though, is there some reason you don't use emscripten's memory views (which are automatically rebound already during updateGlobalBufferAndViews)?

@kg
Copy link
Author

kg commented Apr 19, 2022

We use them, but you don't expose all the views we need. I64, for example. We also have to deal with end users constructing views over the heap (we tell them not to do it, of course), which means eventually we might try to offer our own 'safe view' primitive, and that thing would need to properly respond to heap growth.

@sbc100
Copy link
Collaborator

sbc100 commented Apr 19, 2022

Makes sense.

BTW we have HEAP64 and HEAPU64 these days:

emscripten/src/preamble.js

Lines 249 to 252 in 04c7daa

HEAP64,
/* BigUInt64Array type is not correctly defined in closure
/** not-t@type {!BigUint64Array} */
HEAPU64,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants