niklasvh / base64-arraybuffer

Encode/decode base64 data into ArrayBuffers
MIT License
353 stars 57 forks source link

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

Open greggman opened 4 years ago

greggman commented 4 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

Sounds good?

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

Finesse commented 3 years ago

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