Skip to content

Commit

Permalink
fix: always return uint8arrays (#39)
Browse files Browse the repository at this point in the history
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 at the cost of a performance hit to `Uint8Arrays.allocUnsafe` (see the added `alloc.js` benchmark).

Before:

```
Uint8Arrays.alloc x 1,559,446 ops/sec ±2.00% (79 runs sampled)
Uint8Arrays.allocUnsafe x 5,410,575 ops/sec ±1.11% (90 runs sampled)
new Uint8Array x 1,757,101 ops/sec ±1.85% (79 runs sampled)
Buffer.alloc x 1,691,343 ops/sec ±2.17% (79 runs sampled)
Buffer.allocUnsafe x 6,928,848 ops/sec ±1.18% (89 runs sampled)
Fastest is Buffer.allocUnsafe
```

After:

```
Uint8Arrays.alloc x 1,480,130 ops/sec ±2.64% (80 runs sampled)
Uint8Arrays.allocUnsafe x 4,425,871 ops/sec ±0.91% (91 runs sampled)
new Uint8Array x 1,723,491 ops/sec ±2.62% (75 runs sampled)
Buffer.alloc x 1,697,649 ops/sec ±2.49% (79 runs sampled)
Buffer.allocUnsafe x 6,662,341 ops/sec ±1.25% (88 runs sampled)
Fastest is Buffer.allocUnsafe
```

Fixes #38
  • Loading branch information
achingbrain authored Sep 28, 2022
1 parent 56329d1 commit 017a456
Show file tree
Hide file tree
Showing 11 changed files with 165 additions and 8 deletions.
31 changes: 28 additions & 3 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -43,23 +43,48 @@ 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
steps:
- 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
steps:
- 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
Expand Down
68 changes: 68 additions & 0 deletions benchmarks/alloc.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/* eslint-disable no-console */

/*
$ node benchmarks/alloc.js
$ npx playwright-test benchmarks/alloc.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 })
6 changes: 4 additions & 2 deletions src/alloc.js
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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)
Expand All @@ -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)
Expand Down
3 changes: 2 additions & 1 deletion src/concat.js
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -19,5 +20,5 @@ export function concat (arrays, length) {
offset += arr.length
}

return output
return asUint8Array(output)
}
3 changes: 2 additions & 1 deletion src/from-string.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import bases from './util/bases.js'
import { asUint8Array } from './util/as-uint8array.js'

/**
* @typedef {import('./util/bases').SupportedEncodings} SupportedEncodings
Expand All @@ -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
Expand Down
15 changes: 15 additions & 0 deletions src/util/as-uint8array.js
Original file line number Diff line number Diff line change
@@ -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
}
3 changes: 2 additions & 1 deletion src/xor.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { allocUnsafe } from './alloc.js'
import { asUint8Array } from './util/as-uint8array.js'

/**
* Returns the xor distance between two arrays
Expand All @@ -17,5 +18,5 @@ export function xor (a, b) {
result[i] = a[i] ^ b[i]
}

return result
return asUint8Array(result)
}
16 changes: 16 additions & 0 deletions test/alloc.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
})
10 changes: 10 additions & 0 deletions test/concat.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
})
8 changes: 8 additions & 0 deletions test/from-string.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
})
10 changes: 10 additions & 0 deletions test/xor.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
})

0 comments on commit 017a456

Please sign in to comment.