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

Examine interaction of resizable array buffers with Streams #1248

Open
ricea opened this issue Nov 21, 2022 · 5 comments
Open

Examine interaction of resizable array buffers with Streams #1248

ricea opened this issue Nov 21, 2022 · 5 comments

Comments

@ricea
Copy link
Collaborator

ricea commented Nov 21, 2022

Resizable array buffers are described here:

https://github.com/tc39/proposal-resizablearraybuffer/blob/master/README.md

This issue is to track compatibility issues between resizable array buffers and streams.

@ricea
Copy link
Collaborator Author

ricea commented Nov 21, 2022

My initial assessment based on reading the proposal is that there is no impact on default streams, only byte streams. As far as I can tell, because we detach the buffer in read(view) it can't be resized out from under us.

So my tentative conclusion is that we don't have to change anything.

It might be worth writing some tests once Node supports it.

@MattiasBuelens
Copy link
Collaborator

Should byobReader.read(view) preserve resizability in the returned view? That is: if you pass a resizable buffer to read(view), should the resulting { done, value } also have a resizable value.buffer? I would say it should.

Looking at our current TransferArrayBuffer helper, it looks like it won't have the necessary [[ArrayBufferMaxByteLength]] slot to make it resizable. So at the very least, we'll need to change that helper, so it works more like the proposed ArrayBuffer.prototype.transfer() method.

@ricea
Copy link
Collaborator Author

ricea commented Nov 22, 2022

I hadn't thought of that. I agree that developers will expect to get a resizable buffer back.

@domenic
Copy link
Member

domenic commented Nov 22, 2022

Good catch. I filed tc39/proposal-arraybuffer-transfer#3 on making that easier from a spec perspective.

@domenic
Copy link
Member

domenic commented Nov 23, 2022

I haven't investigated this deeply, but regarding the discussion at tc39/proposal-arraybuffer-transfer#3, I wonder if it might actually be possible to take advantage of resizable buffers in the streams API to make it more pleasant to use? That is, are there technically-backward-incompatible, but probably-real-world-compatible changes we could make, that would let us stop creating so many different ArrayBuffers?

I guess userland streams are the real crux. @syg, is there any way to get this behavior, using spec/implementation primitives introduced by the resizable buffers proposal? (Not necessarily restricted to public JavaScript APIs.)

const ab1 = constructAB();

const ab2 = transferMemoryFrom(ab1);
// at this point ab1 must act as if it's transferred

// ... now developer code manipulates the contents of ab2 ...

switchMemoryBackToAB1();
// at this point ab2 must act as if it's transferred,
// and ab1 must reflect any modifications that were made to the backing memory.

The other area I wonder about, is whether we could/should use the resizable buffers primitives to replace our practice of vending smaller ArrayBufferViews onto the backing ArrayBuffer, to reflect how many bytes we used. Probably not, but maybe worth some thoughts...

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

No branches or pull requests

3 participants