buffer: improve performance of multiple Buffer operations#61871
buffer: improve performance of multiple Buffer operations#61871thisalihassan wants to merge 1 commit intonodejs:mainfrom
Conversation
|
Review requested:
|
d2ba38f to
495feb5
Compare
|
|
||
| function hexSliceToHex(buf, start, end) { | ||
| ensureUint8ArrayToHex(); | ||
| return Uint8ArrayPrototypeToHex( |
There was a problem hiding this comment.
This is a flagged feature and will crash with --no-js-base-64.
There was a problem hiding this comment.
Handled fallback
| void FastSwap16(Local<Value> receiver, | ||
| Local<Value> buffer_obj, | ||
| // NOLINTNEXTLINE(runtime/references) | ||
| FastApiCallbackOptions& options) { | ||
| HandleScope scope(options.isolate); | ||
| ArrayBufferViewContents<char> buffer(buffer_obj); | ||
| CHECK(nbytes::SwapBytes16(const_cast<char*>(buffer.data()), buffer.length())); | ||
| } |
There was a problem hiding this comment.
These fast callbacks are non-identical to the conventional callbacks they shadow.
- The existing callbacks validate their argument and throws to JS if invalid, whereas your fast callbacks hard-crash the process. It might be better to validate in the JS layer, then use the same unwrapping logic on both sides.
- Your fast callback cannot have a different return convention to the conventional callback. You will need to remove the return value from the conventional callback.
| byteOffset, | ||
| encodingsMap.ascii, | ||
| dir), | ||
| indexOfString(buf, val, byteOffset, encodingsMap.latin1, dir), |
There was a problem hiding this comment.
Same behaviour AFAICT, but encodingsMap.ascii seems more appropriate.
There was a problem hiding this comment.
Yes same behaviour IndexOfString only has branches for UCS2, UTF8, and Latin1, Adding an ASCII branch to IndexOfString would just be a duplicate of the Latin1 branch since ASCII is a subset of Latin1.
There was a problem hiding this comment.
That's my point, these are still discrete encodings even though the behaviour here is the same, so this should pass encodingsMap.ascii to the binding and IndexOfString should add a condition to send this down the same path.
| void FastSwap16(Local<Value> receiver, | ||
| Local<Value> buffer_obj, | ||
| // NOLINTNEXTLINE(runtime/references) | ||
| FastApiCallbackOptions& options) { | ||
| HandleScope scope(options.isolate); | ||
| ArrayBufferViewContents<char> buffer(buffer_obj); | ||
| CHECK(nbytes::SwapBytes16(const_cast<char*>(buffer.data()), buffer.length())); | ||
| } |
There was a problem hiding this comment.
Fast callbacks should include debug tracking and call tests.
There was a problem hiding this comment.
Thanks for sharing these I will update the code
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #61871 +/- ##
=======================================
Coverage 89.72% 89.73%
=======================================
Files 675 675
Lines 204806 204861 +55
Branches 39355 39358 +3
=======================================
+ Hits 183761 183825 +64
+ Misses 13330 13321 -9
Partials 7715 7715
🚀 New features to boost your workflow:
|
- copyBytesFrom: calculate byte offsets directly instead of
slicing into an intermediate typed array
- toString('hex'): use V8 Uint8Array.prototype.toHex() builtin
- fill: add single-char ASCII fast path
- indexOf: use indexOfString directly for ASCII encoding
- swap16/32/64: add V8 Fast API functions
495feb5 to
7181059
Compare
Summary
Multiple performance improvements to Buffer operations, verified with benchmarks (15-30 runs, comparing old vs new binaries built from same tree).
Changes
Buffer.copyBytesFrom()(+100-210%)Avoid intermediate
TypedArrayPrototypeSliceallocation by calculating byte offsets directly into the source TypedArray's underlying ArrayBuffer.Buffer.prototype.toString('hex')(+10-30%)Use V8's
Uint8Array.prototype.toHex()builtin (via fast path intoString) instead of the C++hexSlicebinding.Buffer.prototype.fill("t", "ascii")(+26-37%)ASCII
indexOf(+14-46%)Call
indexOfStringdirectly for ASCII encoding instead of first converting the search value to a Buffer viafromStringFastand then callingindexOfBuffer. ASCII and Latin-1 share the same byte values for characters 0-127.swap16/32/64(+3-38%)Add V8 Fast API C++ functions (
FastSwap16/32/64) alongside the existing slow path. Largest gains at len=256 (+35%).Benchmark results
Key results (15-30 runs,
***= p < 0.001):No regressions observed. Full benchmark CSV attached.
compare-all-buffers-final.csv