-
Notifications
You must be signed in to change notification settings - Fork 455
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
[js-api] Integration with the ResizableArrayBuffer and GrowableSharedArrayBuffer proposal #1292
Comments
cc @kripken for thoughts on the design for emscripten-generated code. |
The fields of the Also, there was some discussion that It's great that this feature is being added to JS! To add more context for people here, I've linked below some pretty big toolchain/performance limitations Wasm has been dealing with which would be significantly helped by Resizable/GrowableArrayBuffer integration. |
Ah, good point about non-JS-created memories. I don't have a strong opinion, it's more that I'm not sure how an always-available |
A third path would be another way to signal intent for what kind of buffer you want that's not on the MemoryDescriptor. I'm not fully clued in on the intent of the MemoryDescriptor, so that might be the wrong place to signal intent to begin with. |
Does this have to be the case? Having the same Wasm memory backing both (e.g.) an |
I feel somewhat strongly that aliasing buffers like that on the JS side is a bad idea.
Edit: Contra to #1292 (comment), I guess I do have a strong opinion if the two attributes are supposed to vend aliased-backing-store buffers. :P |
In that case something like #1292 (comment) does seem like the immediate path of least resistance, although if buffer aliasing were to sneak into JS through some other avenue, I'd hope we could relax the throwing behaviour. |
Alternatively, maybe the first vend of a |
Cool, I like the suggestion in #1292 (comment). |
With reference to the OP...
If I'm interpreting the JS proposal correctly, it's mandatory to provide a maximum size, but Since growth can fail even for sizes below this, there's no issue in abstract, but would this approach mess with any engine/code heuristics? EDIT: there was a time when engines aligned on a hard limit of |
That makes sense to me, I'll update the OP.
The engine implication to me is that Wasm memory-backed buffers need special casing. But they do already for special detachment semantics, special resize semantics, etc, so this seem to me that it'll add any more. |
Just to be clear, after some pondering I think that it makes sense for the length in this case to be implementation defined up to any static limit that engines set on memory size which is lower than the spec max. For example, this used to be Generally we've tried to have all Web engines agree to align on such a value, where it's necessary, but it's not documented as part of the "core spec". EDIT: the value is in fact fixed by the JS API, so that should be the number used for the |
Oh, that's gonna be a tricky thing on the JS side. ArrayBuffer sizes aren't BigInts, nor can BigInts be property keys currently. |
I'm reading that as 2^32 (max 65536 pages * 65536 byte page size), yeah? |
AFAIK we haven't explicitly discussed JS integration for the memory64 proposal yet, but that might be something that @aardappel can comment further on at some point.
That's my reading 🎉
BigInt integration (which IIUC was also purely a JS-API change) was handled as a regular proposal, but IIRC it moved through the phases very quickly because there were no core spec changes. Unless anyone comes up with objections here (@kripken dropped a 👍 above) it probably makes sense to add this to the agenda as a poll for phase 1. BigInt integration was proposed quite early on in the life of the committee so people may want to bikeshed a revised procedure for JS-API changes going forward. |
Oh, wrt the new OP edit
The inner bullet just after that line says that the EDIT: Alternatively, it could be a straight exception to subsequently access |
Oh I see, I misunderstood your suggestion. Let me back up and be very explicit: My understanding of the current situation:
Given that, what I thought you were suggesting:
Of course, many other avenues are open, like the ones you suggested in #1292 (comment). I'll be happy to let the CG hash it out. |
Makes sense! |
@conrad-watt from my very minimal understanding of JS, Memory64 doesn't change anything, in the sense that we already had the possibility of The always get turned into BigInts (a decision which I personally find unfortunate) which is causing a ton of churn in e.g. the Emscripten JS bindings, and I suspect elsewhere as well. |
@aardappel But how do you address very large indices? Not all i64s fit inside of a float64. Edit: pesky double negatives |
@syg which "very large indices" for example? 53 bits is.. more than a million 4GB memories all stuffed into one. If we assume that most And this is a dynamically typed language after all, so emitting bigints for those rare >53 bits uses wouldn't be the end of the world. A pragmatic trade-off if you ask me. |
We could probably make |
@aardappel @conrad-watt Thanks for the clarification. Sounds good to me. |
About the idea in #1292 (comment) : I'm a little uneasy about that API surface suggestion, as it seems to contradict the concept that "Getters must not have any observable side effects.". Another design (probably huge overkill) would be to have a custom section indicating how Wasm-allocated memories should be surfaced in JS. Overall, it feels like we're still somewhat early in the design stage here. It would be great if someone could draft a concrete specification PR and rope in other implementers for review. Maybe this is the kind of thing that would benefit from an explainer as well, so it's easier to figure out what's happening without having to trace the arguments in several issue threads. |
@littledan instead of an accessor property, what about an explicit method like EDIT: bikeshedding... maybe the new method puts the
If such a spec PR wouldn't be ready for the next meeting, it might be best to make a phase 1 proposal ASAP anyway, to attract more eyeballs, and then present the PR as part of a phase 2 poll in the subsequent meeting. |
Unfortunately I didn't add it in time to the next meeting before the agenda was filled up. I've added it to the https://github.com/WebAssembly/meetings/blob/master/main/2021/CG-03-30.md, which should also give enough time to make a spec PR. |
My use case is live streaming media. Given a I initially tried to "carry over" the odd float value to the next I then tried Is it possible to clear memory allocated to a What I am uncertain about is if writing values from an existing (If this is not the thread to ask the above questions, kindly refer me to where to ask the questions). |
The ResizableArrayBuffer and GrowableSharedArrayBuffer proposal is a TC39 proposal that is currently stage 2.
There was some discussion of how the proposal might integrate with the Wasm/JS API. It seems better to move the discussion here to get a concrete plan and to get more Wasm eyes on the issue.
As a starting point, I suggest the following changes to the WebAssembly.Memory API:
Memory
will have a newresizableBuffer
attribute.ResizableArrayBuffer
orGrowableSharedArrayBuffer
instance.ArrayBuffer
was vended by thebuffer
attribute on the same instance, thatArrayBuffer
instance will be detached.SharedArrayBuffers
andGrowableSharedArrayBuffer
will alias the same memory.Memory
was created with a maximum size, that size will be reflected on theResizableArrayBuffer
orGrowableSharedArrayBuffer
instance.2^32
.Memory
'sbuffer
attribute will detach anyResizableArrayBuffer
buffers vended by theresizableBuffer
attribute.ResizableArrayBuffer
s gotten viaMemory
'sresizableBuffer
attribute will:transfer()
, since they can't be detached.resize()
calls that are shrinks.resize()
calls that are not in multiples of the wasm page size. (Thanks @conrad-watt)GrowableArrayBuffer
s gotten viaMemory
'sresizableBuffer
attribute will:grow()
calls that are not in multiples of the wasm page size.Thoughts welcome.
(Also, what is the process for bringing such a "normative PR" that's not a full proposal to the CG? Is it just an agenda item?)
The text was updated successfully, but these errors were encountered: