Skip to content

WebAssembly.Memory: length, initial, maximum#1027

Closed
jfbastien wants to merge 5 commits intomainfrom
jfbastien-patch-4
Closed

WebAssembly.Memory: length, initial, maximum#1027
jfbastien wants to merge 5 commits intomainfrom
jfbastien-patch-4

Conversation

@jfbastien
Copy link
Member

As suggested in #1024.

I'll follow-up with a similar suggestion for WebAssembly.Instance, which needs its own discussion about encapsulation as well as multiple memories. WebAssembly.Memory is simpler: there's only one Memory and it's not encapsulated.

As suggested in #1024.

I'll follow-up with a similar suggestion for WebAssembly.Instance, which needs its own discussion about encapsulation as well as multiple memories. WebAssembly.Memory is simpler: there's only one Memory and it's not encapsulated.
@jfbastien
Copy link
Member Author

One thing that remains outstanding is how to identify that these are post-MVP things. We use 🦄 for future features, maybe we could use another marker for post-MVP?

length is trivial to polyfill. initial and maximum aren't, unless one wraps WebAssembly.Memory and WebAssembly.Instance in proxies, as well as all the free functions.

Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to initialize those slots, not just state they exist. (Or, maybe grab them from the internal [[Memory]].

I don't think adding a shortcut for .buffer.length is a good idea. Objects with .length properties in JavaScript are uniformly indexed-access objects. The only reasonable name is .bufferLength, but then, why not .buffer.length?

@jfbastien
Copy link
Member Author

@domenic

This needs to initialize those slots, not just state they exist. (Or, maybe grab them from the internal [[Memory]].

Ah yes! Done.

I don't think adding a shortcut for .buffer.length is a good idea. Objects with .length properties in JavaScript are uniformly indexed-access objects. The only reasonable name is .bufferLength, but then, why not .buffer.length?

That was discussed in #1024. I'm OK with any name that's consistent.

.buffer.length means we'd create an ArrayBuffer only to throw it away, no? Doesn't hurt much, but seems silly?

@domenic
Copy link
Member

domenic commented Mar 30, 2017

.buffer.length means we'd create an ArrayBuffer only to throw it away, no? Doesn't hurt much, but seems silly?

Ah, I didn't realize the ArrayBuffer would be created lazily by implementations. (The spec creates it at creation time, but that's the spec; it's observably equivalent to create it lazily.)

More accurately you'd be creating it, and never throwing it away, since you require m.buffer === m.buffer.

If it is indeed anticipated that people will want to know the buffer length but not ever access the buffer itself, then yeah, adding a bufferByteLength makes sense. (I realize now it should be bufferByteLength instead of bufferLength since buffer is an ArrayBuffer, not a Uint8Array.)

@jfbastien
Copy link
Member Author

More accurately you'd be creating it, and never throwing it away, since you require m.buffer === m.buffer.

Until we detach due to grow or the grow_memory opcode, I believe that true.

If it is indeed anticipated that people will want to know the buffer length but not ever access the buffer itself, then yeah, adding a bufferByteLength makes sense. (I realize now it should be bufferByteLength instead of bufferLength since buffer is an ArrayBuffer, not a Uint8Array.)

I was writing code to massage some Emscripten output and it would be useful to have. People aren't clamoring for it.

bufferByteLength SGTM. Will let @lukewagner chime in.

Copy link
Member

@lukewagner lukewagner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No strong opinion on the length name; just trying to figure out what is the most consistent. bufferByteLength is kinda verbose and I think, for wasm, the Memory is the real owner of the memory--the ArrayBuffer just gets to alias it--and so byteLength seems like the right name if length is wrong.

Probably good to get @rossberg-chromium 's opinion too; he's out on PTO for a bit of time, though.

@domenic
Copy link
Member

domenic commented Mar 31, 2017

Given that conceptual model, byteLength is probably OK; it does make this object kind of take over the territory of ArrayBuffers though. I wonder if it should have been a subclass or something.

@jfbastien
Copy link
Member Author

I've renamed to byteLength and added markers for post-MVP features similar to what we do for feature testing. WDYT?

Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for taking my comments and walking me through things.

@rossberg
Copy link
Member

rossberg commented Apr 3, 2017

maximum and current/length sound good to me, but I'm less convinced about initial. In V8 at least, we don't even remember that information. What would be the use case for "historic" information like that?

As for names, I would prefer either length or currentLength, and maximumLength. That's symmetric, and both those names would be analogously applicable to Table, whereas byteLength is not.

@jfbastien
Copy link
Member Author

maximum and current/length sound good to me, but I'm less convinced about initial. In V8 at least, we don't even remember that information. What would be the use case for "historic" information like that?

It's symmetrical with the WebAssembly.Memory constructor, which I agree a user could just remember, but symmetry is nice. It is, however, not accessible for an instance's module's Memory section without disassembling (and holding onto the binary).

As for names, I would prefer either length or currentLength, and maximumLength. That's symmetric, and both those names would be analogously applicable to Table, whereas byteLength is not.

If we want to be consistent I'd go with one of:

  • current_memory in units of pages, which is what the binary format does
  • byteLength which is what ArrayBuffer does

Going with the corresponding JS API seems more consistent IMO, but I'm happy painting the bikeshed any length.

maximumLength is different from the WebAssembly.Memory constructor, and I don't think it should differ.

@rossberg
Copy link
Member

rossberg commented Apr 3, 2017

@jfbastien, re consistency: the right feature to look at would be neither of the above IMO but the "resizable limits" in the import/export types of memories/tables, because that's pretty much exactly what the current/maximum methods would be reflecting on a memory/table object and what is relevant for linking.

Fair point about the different units, but that doesn't need to affect naming. The constructors and grow methods of tables/memories already use different units in the same way.

Not convinced by the symmetry argument regarding initial. It is a very strange method to have IMO -- you cannot ask a JS array what size it was created with either. Furthermore, the constructor does not take a current length, so I'm not even sure what consistency would be established there. (Maybe it was a mistake to name the constructor param initial instead of just length, but that should not be the sole motivation for adding weird API.)

@flagxor
Copy link
Member

flagxor commented Apr 21, 2017

I agree with @rossberg-chromium that including initial is odd. This seems a pretty unusual thing to track for a memory buffer.
I'm indifferent on naming choice of byteLength / length but buy the utility of having a way to check size that doesn't require creating an ArrayBuffer.

I think a way to access the initial and maximum size of a memory import on a Modle is definitely useful too (possibly more so). Should we consider that?

@jfbastien
Copy link
Member Author

I agree with @rossberg-chromium that including initial is odd. This seems a pretty unusual thing to track for a memory buffer.

Yeah let's do it as part of #1046 in Module. Does this sound good?

I'm indifferent on naming choice of byteLength / length but buy the utility of having a way to check size that doesn't require creating an ArrayBuffer.

Same. We're inconsistent with one thing or another.

I think a way to access the initial and maximum size of a memory import on a Modle is definitely useful too (possibly more so). Should we consider that?

Yes, #1046.

@jfbastien
Copy link
Member Author

Updated, PTAL.

JS.md Outdated
If `this` is not a `WebAssembly.Memory`, a [`TypeError`](https://tc39.github.io/ecma262/#sec-native-error-types-used-in-this-standard-typeerror)
is thrown. Otherwise return `M.[[BufferObject]]`.

### `WebAssembly.Memory.prototype.length`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still quite opposed to length for things without indexed properties 0, 1, 2, ..., length - 1, in JavaScript APIs. If you're indifferent between length and byteLength, please pick byteLength :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. That's what I had but @rossberg-chromium disliked it. I'll put it back!

@rossberg
Copy link
Member

@domenic, we need a name that works consistently for both memories and tables, and unfortunately byteLength does not.

@jfbastien, how about size?

@jfbastien
Copy link
Member Author

@domenic, we need a name that works consistently for both memories and tables, and unfortunately byteLength does not.

Why do we need this property?

@jfbastien, how about size?

If we indeed need that property then size is as good as any.

@rossberg
Copy link
Member

@jfbastien, because tables share the exact same limits properties, so should expose them in a consistent manner. If length is verboten, then we'll have to find another name for it for tables anyway, so could just as well use that for memories, too.

@jfbastien
Copy link
Member Author

@jfbastien, because tables share the exact same limits properties, so should expose them in a consistent manner.

Can you clarify what limit property you mean? I don't understand what the similarity is.

@rossberg
Copy link
Member

Tables can grow in the same way as memories, and they also have an initial size, a current size, and an optional maximum size (see e.g. the use of resizable_limits for both in the design docs). The only difference is that these sizes are measured in number of elements instead of pages.

@jfbastien
Copy link
Member Author

Tables can grow in the same way as memories, and they also have an initial size, a current size, and an optional maximum size (see e.g. the use of resizable_limits for both in the design docs). The only difference is that these sizes are measured in number of elements instead of pages.

Ah OK, so IIUC your point is: presence of initial and maximum plus growth, regardless of unit, is what should guide naming. Unit is irrelevant.

Is this accurate?

Two questions I think will help resolve this:

  1. Is this something we've done in the WebAssembly JS API elsewhere (either what you suggest or the opposite)?
  2. What do other web platform APIs do in these circumstances?

@rossberg
Copy link
Member

Well, in the API, we already have the same symmetry in the Table vs Memory constructor arguments (both taking an object with initial and maximum properties. And it is present throughout the Wasm design and semantics, so I would expect that to be reflected accordingly.

@jfbastien
Copy link
Member Author

Well, in the API, we already have the same symmetry in the Table vs Memory constructor arguments (both taking an object with initial and maximum properties. And it is present throughout the Wasm design and semantics, so I would expect that to be reflected accordingly.

True. Your argument is then: we've ignored unit before, and we should therefore continue ignoring it. Right?

Seems sensible.

@domenic thoughts?

@lukewagner
Copy link
Member

Table already has Table.prototype.length so, if we want to be symmetric (which I like), we should have Memory.prototype.length and, to be symmetric with initial, maximum and grow, this must return the current number of pages. But of course it's quite useful to have number of bytes so I think it would be reasonable to additionally have Memory.prototype.byteLength that saved the user the *64*1024 or .buffer.

@domenic
Copy link
Member

domenic commented May 12, 2017

Oh... if we've already decided to break the rule on .length only being for indexed objects, then I guess consistency argues we should continue to do so... it's a shame that wasn't caught earlier :(

Copy link

@littledan littledan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the current state of this proposal? Should I add it to the JS API specification?

Return a new `WebAssembly.Memory` instance with
[[Memory]] set to `m`,
[[BufferObject]] set to `buffer`, and
[[Maximum]] set to `undefined` if `maximum` is `None` or `maximum` otherwise.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the motivation for storing the maximum here, rather than looking it up in the store?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean "in the store"?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

He means that the memory instance m (living in the Wasm abstract store) already has this information, so the internal slot is redundant.

Let `T` be the `this` value. If `T` is not a `WebAssembly.Memory`, a [`TypeError`](https://tc39.github.io/ecma262/#sec-native-error-types-used-in-this-standard-typeerror)
is thrown.

Return `T.[[BufferObject]].[[ArrayBufferByteLength]]`.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the motivation for this feature, when a user can do memory.buffer.byteLength? Is it more optimizable if you never access the ArrayBuffer, or something like that?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah creating the array buffer is expensive-ish. We can optimize it away, but that's being extra silly IMO.

@jfbastien
Copy link
Member Author

What's the current state of this proposal? Should I add it to the JS API specification?

That would be great, though I'd advise bringing it up at a CG meeting (either in-person or VC).

@rossberg
Copy link
Member

We deliberately chose to make this post-MVP, so we should not rush it into the MVP spec now all of a sudden. There is a bit of a design space around how to provide this information and fit it nicely with #1046, which would expose equivalent information in other places. I think these features need to be designed together.

@sunfishcode
Copy link
Member

If anyone wishes to pursue this feature, please make a new proposal to the CG.

@sunfishcode sunfishcode deleted the jfbastien-patch-4 branch February 22, 2024 21:34
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

Successfully merging this pull request may close these issues.

7 participants