From e311d723c03bbf656e69544f287237b26e87645d Mon Sep 17 00:00:00 2001 From: achingbrain Date: Wed, 14 Sep 2022 11:49:09 +0100 Subject: [PATCH 1/2] fix: always return uint8arrays 3.1.0 incorporated internal refactors to use node `Buffer`s where possible to increase performance when `alloc`ing new byte arrays. It then returns those `Buffer`s but the problem is `Buffer` is not completely compatible with `Uint8Array` as some methods with the same name behave differently. We can convert a `Buffer` to a `Uint8Array` without copying it by using the 3-arg `Uint8Array` constructor so we should do that to retain the performance characteristics of `Buffer` when `alloc`ing but the compatibility of returning vanilla `Uint8Array`s. --- benchmarks/alloc.js | 68 +++++++++++++++++++++++++++++++++++++++ src/alloc.js | 6 ++-- src/concat.js | 3 +- src/from-string.js | 3 +- src/util/as-uint8array.js | 15 +++++++++ src/xor.js | 3 +- test/alloc.spec.js | 16 +++++++++ test/concat.spec.js | 10 ++++++ test/from-string.spec.js | 8 +++++ test/xor.spec.js | 10 ++++++ 10 files changed, 137 insertions(+), 5 deletions(-) create mode 100644 benchmarks/alloc.js create mode 100644 src/util/as-uint8array.js diff --git a/benchmarks/alloc.js b/benchmarks/alloc.js new file mode 100644 index 0000000..8cb2d33 --- /dev/null +++ b/benchmarks/alloc.js @@ -0,0 +1,68 @@ +/* eslint-disable no-console */ + +/* +$ node benchmarks/to-string.js +$ npx playwright-test benchmarks/to-string.js --runner benchmark +*/ + +import Benchmark from 'benchmark' +import { alloc, allocUnsafe } from '../src/alloc.js' + +const LENGTH = 1024 + +function checkAlloc (arr) { + return arr.byteLength !== LENGTH +} + +const suite = new Benchmark.Suite() + +suite + .add('Uint8Arrays.alloc', () => { + const res = alloc(LENGTH) + + if (checkAlloc(res)) { + throw new Error('Alloc failed') + } + }) + .add('Uint8Arrays.allocUnsafe', () => { + const res = allocUnsafe(LENGTH) + + if (checkAlloc(res)) { + throw new Error('Alloc failed') + } + }) + .add('new Uint8Array', () => { + const res = new Uint8Array(LENGTH) + + if (checkAlloc(res)) { + throw new Error('Alloc failed') + } + }) + +if (globalThis.Buffer != null) { + suite.add('Buffer.alloc', function () { + const res = globalThis.Buffer.alloc(LENGTH) + + if (checkAlloc(res)) { + throw new Error('Alloc failed') + } + }) + suite.add('Buffer.allocUnsafe', function () { + const res = globalThis.Buffer.allocUnsafe(LENGTH) + + if (checkAlloc(res)) { + throw new Error('Alloc failed') + } + }) +} + +suite + // add listeners + .on('cycle', (event) => { + console.log(String(event.target)) + }) + .on('complete', function () { + console.log('Fastest is ' + this.filter('fastest').map('name')) + }) + // run async + .run({ async: true }) diff --git a/src/alloc.js b/src/alloc.js index 0a69a3d..1cf0b06 100644 --- a/src/alloc.js +++ b/src/alloc.js @@ -1,3 +1,5 @@ +import { asUint8Array } from './util/as-uint8array.js' + /** * Returns a `Uint8Array` of the requested size. Referenced memory will * be initialized to 0. @@ -7,7 +9,7 @@ */ export function alloc (size = 0) { if (globalThis.Buffer != null && globalThis.Buffer.alloc != null) { - return globalThis.Buffer.alloc(size) + return asUint8Array(globalThis.Buffer.alloc(size)) } return new Uint8Array(size) @@ -23,7 +25,7 @@ export function alloc (size = 0) { */ export function allocUnsafe (size = 0) { if (globalThis.Buffer != null && globalThis.Buffer.allocUnsafe != null) { - return globalThis.Buffer.allocUnsafe(size) + return asUint8Array(globalThis.Buffer.allocUnsafe(size)) } return new Uint8Array(size) diff --git a/src/concat.js b/src/concat.js index 67464a5..2db94d9 100644 --- a/src/concat.js +++ b/src/concat.js @@ -1,4 +1,5 @@ import { allocUnsafe } from './alloc.js' +import { asUint8Array } from './util/as-uint8array.js' /** * Returns a new Uint8Array created by concatenating the passed ArrayLikes @@ -19,5 +20,5 @@ export function concat (arrays, length) { offset += arr.length } - return output + return asUint8Array(output) } diff --git a/src/from-string.js b/src/from-string.js index 752fcc0..7f88286 100644 --- a/src/from-string.js +++ b/src/from-string.js @@ -1,4 +1,5 @@ import bases from './util/bases.js' +import { asUint8Array } from './util/as-uint8array.js' /** * @typedef {import('./util/bases').SupportedEncodings} SupportedEncodings @@ -23,7 +24,7 @@ export function fromString (string, encoding = 'utf8') { } if ((encoding === 'utf8' || encoding === 'utf-8') && globalThis.Buffer != null && globalThis.Buffer.from != null) { - return globalThis.Buffer.from(string, 'utf8') + return asUint8Array(globalThis.Buffer.from(string, 'utf-8')) } // add multibase prefix diff --git a/src/util/as-uint8array.js b/src/util/as-uint8array.js new file mode 100644 index 0000000..46ce632 --- /dev/null +++ b/src/util/as-uint8array.js @@ -0,0 +1,15 @@ + +/** + * To guarantee Uint8Array semantics, convert nodejs Buffers + * into vanilla Uint8Arrays + * + * @param {Uint8Array} buf + * @returns {Uint8Array} + */ +export function asUint8Array (buf) { + if (globalThis.Buffer != null) { + return new Uint8Array(buf.buffer, buf.byteOffset, buf.byteLength) + } + + return buf +} diff --git a/src/xor.js b/src/xor.js index 82117dd..cb9fb50 100644 --- a/src/xor.js +++ b/src/xor.js @@ -1,4 +1,5 @@ import { allocUnsafe } from './alloc.js' +import { asUint8Array } from './util/as-uint8array.js' /** * Returns the xor distance between two arrays @@ -17,5 +18,5 @@ export function xor (a, b) { result[i] = a[i] ^ b[i] } - return result + return asUint8Array(result) } diff --git a/test/alloc.spec.js b/test/alloc.spec.js index ad07a2d..55932f6 100644 --- a/test/alloc.spec.js +++ b/test/alloc.spec.js @@ -22,4 +22,20 @@ describe('Uint8Array alloc', () => { expect(allocUnsafe(size)).to.have.property('byteLength', size) }) + + it('alloc returns Uint8Array', () => { + const a = alloc(10) + const slice = a.slice() + + // node slice is a copy operation, Uint8Array slice is a no-copy operation + expect(slice.buffer).to.not.equal(a.buffer) + }) + + it('allocUnsafe returns Uint8Array', () => { + const a = allocUnsafe(10) + const slice = a.slice() + + // node slice is a copy operation, Uint8Array slice is a no-copy operation + expect(slice.buffer).to.not.equal(a.buffer) + }) }) diff --git a/test/concat.spec.js b/test/concat.spec.js index 8156ac7..8675dd4 100644 --- a/test/concat.spec.js +++ b/test/concat.spec.js @@ -35,4 +35,14 @@ describe('Uint8Array concat', () => { expect(concat([a, b], 8)).to.deep.equal(c) }) + + it('concat returns Uint8Array', () => { + const a = Uint8Array.from([0, 1, 2, 3]) + const b = [4, 5, 6, 7] + const c = concat([a, b]) + const slice = c.slice() + + // node slice is a copy operation, Uint8Array slice is a no-copy operation + expect(slice.buffer).to.not.equal(c.buffer) + }) }) diff --git a/test/from-string.spec.js b/test/from-string.spec.js index ac2a81b..23f8523 100644 --- a/test/from-string.spec.js +++ b/test/from-string.spec.js @@ -52,4 +52,12 @@ describe('Uint8Array fromString', () => { // @ts-expect-error 'derp' is not a valid encoding expect(() => fromString(str, 'derp')).to.throw(/Unsupported encoding/) }) + + it('fromString returns Uint8Array', () => { + const a = fromString('derp') + const slice = a.slice() + + // node slice is a copy operation, Uint8Array slice is a no-copy operation + expect(slice.buffer).to.not.equal(a.buffer) + }) }) diff --git a/test/xor.spec.js b/test/xor.spec.js index 81059ed..1f70b3d 100644 --- a/test/xor.spec.js +++ b/test/xor.spec.js @@ -24,4 +24,14 @@ describe('Uint8Array xor', () => { expect(xor(a, b)).to.deep.equal(Uint8Array.from([0, 0])) }) + + it('xors returns Uint8Array', () => { + const a = Uint8Array.from([1, 1]) + const b = Uint8Array.from([1, 1]) + const c = xor(a, b) + const slice = c.slice() + + // node slice is a copy operation, Uint8Array slice is a no-copy operation + expect(slice.buffer).to.not.equal(c.buffer) + }) }) From 22b24e836a48980a457466a250f81fae2c1a9035 Mon Sep 17 00:00:00 2001 From: achingbrain Date: Wed, 14 Sep 2022 12:15:37 +0100 Subject: [PATCH 2/2] chore: get chrome coverage --- .github/workflows/main.yml | 31 ++++++++++++++++++++++++++++--- benchmarks/alloc.js | 4 ++-- 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 0ca9e95..b984eda 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -43,7 +43,16 @@ jobs: - uses: actions/checkout@v2 - uses: microsoft/playwright-github-action@v1 - run: npm install - - run: npx aegir test -t browser -t webworker --bail + - run: npx aegir test -t browser --bail --cov + - uses: codecov/codecov-action@v1 + test-chrome-webworker: + needs: check + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - uses: microsoft/playwright-github-action@v1 + - run: npm install + - run: npx aegir test -t webworker --bail test-firefox: needs: check runs-on: ubuntu-latest @@ -51,7 +60,15 @@ jobs: - uses: actions/checkout@v2 - uses: microsoft/playwright-github-action@v1 - run: npm install - - run: npx aegir test -t browser -t webworker --bail -- --browser firefox + - run: npx aegir test -t browser --bail -- --browser firefox + test-firefox-webworker: + needs: check + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - uses: microsoft/playwright-github-action@v1 + - run: npm install + - run: npx aegir test -t webworker --bail -- --browser firefox test-webkit: needs: check runs-on: ubuntu-latest @@ -59,7 +76,15 @@ jobs: - uses: actions/checkout@v2 - uses: microsoft/playwright-github-action@v1 - run: npm install - - run: npx aegir test -t browser -t webworker --bail -- --browser webkit + - run: npx aegir test -t browser --bail -- --browser webkit + test-webkit-webworker: + needs: check + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - uses: microsoft/playwright-github-action@v1 + - run: npm install + - run: npx aegir test -t webworker --bail -- --browser webkit test-electron-main: needs: check runs-on: ubuntu-latest diff --git a/benchmarks/alloc.js b/benchmarks/alloc.js index 8cb2d33..ee2692b 100644 --- a/benchmarks/alloc.js +++ b/benchmarks/alloc.js @@ -1,8 +1,8 @@ /* eslint-disable no-console */ /* -$ node benchmarks/to-string.js -$ npx playwright-test benchmarks/to-string.js --runner benchmark +$ node benchmarks/alloc.js +$ npx playwright-test benchmarks/alloc.js --runner benchmark */ import Benchmark from 'benchmark'