Performance regression in Buffer.concat

[wemeetagain]

Version

v18.12.1

Platform

Linux ruby 5.15.0-53-generic #59-Ubuntu SMP Mon Oct 17 18:53:30 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

Buffer

What steps will reproduce the bug?

Run this under node v18 vs node v16

const randomBuffer = (length) => Buffer.from(Array.from({length}, () => Math.random() * 255))

const testCases = [
 {bytesPerBuffer: 32, numBuffers: 10},
]

for (const {bytesPerBuffer, numBuffers} of testCases) {
 const results = []
 for (let i = 0; i < 100_000; i++) {
 const buffers = Array.from({length: numBuffers}, () => randomBuffer(bytesPerBuffer))
 const before = performance.now()
 Buffer.concat(buffers)
 const after = performance.now()
 results.push(after - before)
 }
 const avgTime = results.reduce((a, b) => a + b, 0) / results.length
 console.log(`concat ${numBuffers} buffers, ${bytesPerBuffer} each: ${avgTime}s`)
}

How often does it reproduce? Is there a required condition?

Always reproduceable

What is the expected behavior?

Expected behavior is that there is no performance regression.

What do you see instead?

Buffer.concat is ~2.5x slower after node v16 (v17 and beyond)

Additional information

No response

[RafaelGSS]

HI @wemeetagain. I've run it locally and it seems fixed in v19. See below:

v14

concat 10 buffers, 32 each: 0.00033s

v16

concat 10 buffers, 32 each: 0.00044s

v18

concat 10 buffers, 32 each: 0.00114s

v19

concat 10 buffers, 32 each: 0.0004s

---

Note: I haven't investigated the cause, but, I'd wait for the next v18 release and compare.

[krk]

I bisected with the help of nvm, found that 17.2.0 introduced the regression. Then profiling with perf using this modified reproducer:

const randomBuffer = (length) => Buffer.from(Array.from({ length }, () => Math.random() * 255))

const ranBuf = randomBuffer(32);

const results = []
const buffers = Array.from({ length: 10 }, () => ranBuf)

const before = performance.now();
for (let i = 0; i < 10000000; i++) {
 Buffer.concat(buffers)
}
const after = performance.now()
console.log(after - before);

17.1.0 (good)

 28,82% node node [.] Builtins_TypedArrayPrototypeSet 
 6,52% node node [.] Builtins_CreateTypedArray 
 4,11% node [JIT] tid 6479 [.] 0x00007f307e848d70 
 3,53% node [JIT] tid 6479 [.] 0x00007f307e848d2e 
 2,92% node libc-2.26.so [.] __memmove_avx_unaligned_erms 
 2,66% node node [.] Builtins_LoadGlobalIC 
 1,71% node libc-2.26.so [.] _int_malloc 
 1,54% node [JIT] tid 6479 [.] 0x00007f307e848d3d 
 1,52% node [JIT] tid 6479 [.] 0x00007f307e848a46 
 1,43% node libc-2.26.so [.] malloc
...
 0,02% node node [.] Builtins_LoadIC 
 0,08% node node [.] Builtins_Call_ReceiverIsAny 
 0,00% node node [.] Builtins_TypedArrayPrototypeLength 
 0,81% node node [.] Builtins_CallFunction_ReceiverIsAny 

 Builtins_LoadICTrampoline NOT sampled.

17.2.0 (bad)

 32,32% node node [.] Builtins_LoadIC
 13,60% node node [.] Builtins_Call_ReceiverIsAny
 12,57% node node [.] Builtins_TypedArrayPrototypeLength
 9,43% node node [.] Builtins_CallFunction_ReceiverIsAny
 8,66% node node [.] Builtins_TypedArrayPrototypeSet
 1,82% node node [.] Builtins_CreateTypedArray
 1,69% node node [.] Builtins_LoadICTrampoline
 0,96% node [JIT] tid 6491 [.] 0x00007f7ab7048fa0
 0,93% node node [.] Builtins_LoadGlobalIC
 0,85% node libc-2.26.so [.] __memmove_avx_unaligned_erms
 0,59% node libc-2.26.so [.] _int_malloc
 0,38% node libc-2.26.so [.] malloc

In 17.2.0, Builtins_* dominate the call stacks, pointing to a v8 issue.

Further bisecting with git identifies #40488, which updates v8 from 9.5.172.25 to 9.6.180.14 as the PR when this regression was introduced.

[spollack]

Question: is the perf fix likely to be picked up for node 18.x? or will we have to wait for 20.x to have the fix in an LTS? Thank you.

[wemeetagain]

appears to be resolved by node 20 (also 22 is good)