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

buffer: optimize asciiWrite for short strings #53071

Closed
wants to merge 7 commits into from

Conversation

ronag
Copy link
Member

@ronag ronag commented May 20, 2024

Based on benchmark:

import { bench, group, run } from 'mitata'

const BUF = Buffer.allocUnsafe(64).fill(88)

function asciiWrite(str) {
  BUF.asciiWrite(str, 0)
}

function bufWrite(buf) {
  BUF.set(buf)
}

function loopWrite(src) {
  for (let n = 0; n < src.length; n++) {
    BUF[n] = src.charCodeAt(n)
  }
}

const str8 = '01234567'
const buf8 = Buffer.from(str8)
const str16 = '0123456789abcdef'
const buf16 = Buffer.from(str16)
const str32 = '0123456789abcdef'.repeat(2)
const buf32 = Buffer.from(str32)
const str64 = '0123456789abcdef'.repeat(4)
const buf64 = Buffer.from(str64)

group('8', () => {
  bench('asciiWrite', () => asciiWrite(str8))
  bench('loopWrite', () => loopWrite(str8))
  bench('bufWrite', () => bufWrite(buf8))
})
group('16', () => {
  bench('asciiWrite', () => asciiWrite(str16))
  bench('loopWrite', () => loopWrite(str16))
  bench('bufWrite', () => bufWrite(buf16))
})
group('32', () => {
  bench('asciiWrite', () => asciiWrite(str32))
  bench('loopWrite', () => loopWrite(str32))
  bench('bufWrite', () => bufWrite(buf32))
})
group('64', () => {
  bench('asciiWrite', () => asciiWrite(str64))
  bench('loopWrite', () => loopWrite(str64))
  bench('bufWrite', () => bufWrite(buf64))
})

await run()
cpu: Apple M2 Pro
runtime: node v22.2.0 (arm64-darwin)

benchmark       time (avg)             (min … max)       p75       p99      p999
-------------------------------------------------- -----------------------------
• 8
-------------------------------------------------- -----------------------------
asciiWrite   41.99 ns/iter     (39.88 ns … 247 ns)  41.65 ns  79.55 ns    182 ns
loopWrite     7.12 ns/iter    (6.94 ns … 70.11 ns)   7.04 ns   8.99 ns  21.55 ns
bufWrite      9.87 ns/iter      (9.56 ns … 219 ns)   9.75 ns  13.67 ns   18.8 ns

summary for 8
  loopWrite
   1.39x faster than bufWrite
   5.9x faster than asciiWrite

• 16
-------------------------------------------------- -----------------------------
asciiWrite   41.33 ns/iter     (39.86 ns … 333 ns)  41.32 ns  51.39 ns    149 ns
loopWrite    12.99 ns/iter     (12.49 ns … 125 ns)   12.7 ns     20 ns  45.02 ns
bufWrite     11.39 ns/iter    (9.97 ns … 3'374 ns)  10.23 ns  16.44 ns    101 ns

summary for 16
  bufWrite
   1.14x faster than loopWrite
   3.63x faster than asciiWrite

• 32
-------------------------------------------------- -----------------------------
asciiWrite   40.84 ns/iter     (39.86 ns … 164 ns)     41 ns  47.79 ns  61.28 ns
loopWrite    25.89 ns/iter   (24.25 ns … 1'722 ns)   24.8 ns  47.24 ns    256 ns
bufWrite     10.95 ns/iter   (10.74 ns … 75.05 ns)  10.86 ns  13.27 ns  19.47 ns

summary for 32
  bufWrite
   2.36x faster than loopWrite
   3.73x faster than asciiWrite

• 64
-------------------------------------------------- -----------------------------
asciiWrite   41.74 ns/iter     (40.77 ns … 244 ns)  41.91 ns  47.89 ns     85 ns
loopWrite    60.41 ns/iter   (54.73 ns … 3'905 ns)  56.52 ns  97.35 ns  2'027 ns
bufWrite      9.81 ns/iter      (9.56 ns … 375 ns)   9.66 ns     13 ns  19.57 ns

summary for 64
  bufWrite
   4.25x faster than asciiWrite
   6.16x faster than loopWrite

@nodejs-github-bot nodejs-github-bot added buffer Issues and PRs related to the buffer subsystem. needs-ci PRs that need a full CI run. labels May 20, 2024
Copy link
Member

@lemire lemire left a comment

Choose a reason for hiding this comment

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

This should call v8's WriteOneByte more or less directly, so it is hard to beat (for sufficiently long strings).

@lemire
Copy link
Member

lemire commented May 20, 2024

I don't undestand the test failure.

lib/internal/buffer.js Outdated Show resolved Hide resolved
@@ -962,6 +962,16 @@ class FastBuffer extends Uint8Array {
}
}

function _asciiWrite(string, offset, length) {
if (length <= 32) {
Copy link
Member Author

Choose a reason for hiding this comment

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

I think some error checking might be missing here.

@ronag ronag closed this May 20, 2024
@ronag
Copy link
Member Author

ronag commented May 20, 2024

This was more work than I initially assumed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
buffer Issues and PRs related to the buffer subsystem. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants