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

feat: make Array.buffer public #1815

Closed
wants to merge 1 commit into from

Conversation

yjhmelody
Copy link
Contributor

ArrayBuffer is often needed to pass arrays when interacting with host function, but only Array does not expose buffer properties. It is unacceptable to get the copy overhead of ArrayBuffer every time it is converted to other Array types.

@MaxGraey
Copy link
Member

MaxGraey commented Apr 19, 2021

Usually you don't need Buffer. You're need pointer to internal array data right? In this case Array and TypedArray provide dataStart field

@yjhmelody
Copy link
Contributor Author

But they are private, although the private modifier is currently invalid, but I hope that a public field can do this (and is documented), dataStart is also private

@MaxGraey
Copy link
Member

MaxGraey commented Apr 19, 2021

But they are private, although the private modifier is currently invalid, but I hope that a public field can do this (and is documented), dataStart is also private

"dataStart" is public but unsafe.

basically all FFI interfaces which use AssemblyScript (Fastly/lucet, NEAR/wasmer and just standard WASI) use dataStart

@MaxGraey
Copy link
Member

@@ -33,7 +33,7 @@ export class Array<T> {
// `dataStart` (equals `buffer`) and `byteLength` (equals computed `buffer.byteLength`), but the
// block is 16 bytes anyway so it's fine to have a couple extra fields in there.

private buffer: ArrayBuffer;
readonly buffer: ArrayBuffer;
private dataStart: usize;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@MaxGraey Isn't it private now?

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, good point. In ArrayBufferView it's currently public. So I guess better make this public as well and add @unsafe decorator. @dcodeIO wdyt?

@yjhmelody
Copy link
Contributor Author

For example:
https://github.com/jedisct1/as-wasi/blob/master/assembly/as-wasi.ts#L295

Do you mean always changetype first?

@MaxGraey
Copy link
Member

Do you mean always changetype first?

Basically you could mimic Array to ArrayBufferView and got access to dataStart as changetype<ArrayBufferView>(array).dataStart

@yjhmelody
Copy link
Contributor Author

yjhmelody commented Apr 19, 2021

Do you mean always changetype first?

Basically you could mimic Array to ArrayBufferView and got access to dataStart as changetype<ArrayBufferView>(array).dataStart

Well, I have a new question now. What is the point of storing both buffer and dataStart in ArrayBufferView? dataStart is equivalent to buffer everywhere. Sorry, what's the meaning of this value?

@MaxGraey
Copy link
Member

Well, I have a new question now. What is the point of storing both buffer and dataStart in ArrayBufferView? dataStart is equivalent to buffer everywhere. Sorry, what's the meaning of this value?

ArrayBufferView is basic class for TypedArray which have also byteOffset so dataStart provide fast access and equivalent to changetype<usize>(typedArr.buffer) + typedArr.byteOffset

@yjhmelody
Copy link
Contributor Author

Well, it seems that the reason is quite complicated

@yjhmelody
Copy link
Contributor Author

yjhmelody commented Apr 20, 2021

Do you mean always changetype first?

Basically you could mimic Array to ArrayBufferView and got access to dataStart as changetype<ArrayBufferView>(array).dataStart

let data_buf_in = changetype<ArrayBufferView>(data).dataStart;

dataStart should be documented in index.d.ts and website.

@MaxGraey
Copy link
Member

dataStart now public in Array after #1828

@MaxGraey MaxGraey closed this Apr 30, 2021
@yjhmelody yjhmelody deleted the Array-buffer branch May 8, 2021 03:53
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.

2 participants