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

Accept a Uint8Array so as not to waste memory and time? #22

Open
greggman opened this issue Oct 6, 2020 · 5 comments
Open

Accept a Uint8Array so as not to waste memory and time? #22

greggman opened this issue Oct 6, 2020 · 5 comments

Comments

@greggman
Copy link

greggman commented Oct 6, 2020

As it is, if you have some Uint8Array that is a view into a larger ArrayBuffer then a copy of the data is made in encode because new Uint8Array(someOtherUint8Array) makes new ArrayBuffer with a copy of the data. Where as you could just use the Uint8Array passed in?

Basically this would change

  exports.encode = function(arraybuffer) {
    var bytes = new Uint8Array(arraybuffer),

To something like

  exports.encode = function(arraybuffer) {
    var bytes = (arrayBuffer instanceof Uint8Array || arrayBuffer instanceof Uint8ClampArray)
            ? arrayBuffer
            : new Uint8Array(arraybuffer),


@Finesse
Copy link

Finesse commented Mar 23, 2021

Where does the copy appear? The following code doesn't copy the array content:

const array = new Uint8Array([45, 69, 80]);
const buffer = array.buffer;
const uintView = new Uint8Array(buffer);

Or are you talking about memory and time spending on creating a Uint8Array reference to the buffer? Isn't it too small compared to the rest part of encode?

@greggman
Copy link
Author

greggman commented Mar 23, 2021

Here's an example

const response = await fetch(someURLToData);
const dataThatContainsMoreThanJustTheDataIWantToEncode = await response.arrayBuffer();

now I only want to pass a portion of dataThatContainsMoreThanJustTheDataIWantToEncode

There's no way to do it. I want to pass in a TypedArray view on the arrayBuffer but the API doesn't take a "view", it only takes the entire buffer. In other words, the data I want to encode is

// create a view to a portion of the data, not a copy
const dataToEncode = new Uint8Array(
   dataThatContainsMoreThanJustTheDataIWantToEncode.buffer,
   offsetToDataToEncode,
   lengthOfDataToEncode);

But the current API requires I copy that to a new arraybuffer

I want to do this

const dataToEncode = new Uint8Array(
   dataThatContainsMoreThanJustTheDataIWantToEncode.buffer,
   offsetToDataToEncode,
   lengthOfDataToEncode);
const b64 = encode(dataToEncode);   // error, api doesn't take a Uint8Array

but I can't. I also can't do this

const dataToEncode = new Uint8Array(
   dataThatContainsMoreThanJustTheDataIWantToEncode.buffer,
   offsetToDataToEncode,
   lengthOfDataToEncode);
const b64 = encode(dataToEncode.buffer);  // error, API will read the entire buffer, not just the portion I wanted to encode

So I'm forced to make a copy

// this makes a copy, not a view
const dataToEncode = dataThatContainsMoreThanJustTheDataIWantToEncode.slice(
   offsetToDataToEncode, offsetToDataToEncode + lengthOfDataToEncode);
// pass the copy
const b64 = encode(dataToEncode); 

I want to avoid that copy

@Finesse
Copy link

Finesse commented Mar 23, 2021

It makes sense, thanks. But I'd suggest a more universal solution:

function getUint8Array(bufferOrView) {
  if (bufferOrView instanceof ArrayBuffer) {
    return new Uint8Array(bufferOrView);
  }
  if (ArrayBuffer.isView(bufferOrView)) {
    return new Uint8Array(bufferOrView.buffer, bufferOrView.byteOffset, bufferOrView.byteLength);
  }
  throw new TypeError('Unsupported argument');
}

exports.encode = function(arraybuffer) {
    var bytes = getUint8Array(arraybuffer);
    // ...
}

It can bloat the code too much. This solution would be suitable too:

exports.encode = function(arraybuffer, byteOffset, byteLength) {
    var bytes = new Uint8Array(arraybuffer, byteOffset, byteLength);
    // ...
}

Or this:

exports.encode = function(bytes) {
    // Can be presented only in the type declarations
    if (!(bytes instanceof Uint8Array || bytes instanceof Uint8ClampArray)) {
      throw new TypeError('Unsupported argument');
    }
    // ...
}

@greggman
Copy link
Author

@Finesse
Copy link

Finesse commented Mar 23, 2021

Sounds good?

It sounds good to me. But I'm not a maintainer of this repository, so I can't change the code.

BTW: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/ArrayBuffer/isView

Nice notice, I've updated my example, thanks

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

2 participants