stream: move WHATWG byte-stream helpers to C++ by mcollina · Pull Request #63570 · nodejs/node

mcollina · 2026-05-25T21:10:16Z

Implement five defensive helpers from lib/internal/webstreams/util.js in a new internal binding (src/node_webstreams.cc) using v8::ArrayBufferView / v8::ArrayBuffer APIs directly:

arrayBufferViewGet{Buffer,ByteLength,ByteOffset}
canCopyArrayBuffer
cloneAsUint8Array

The previous JS versions used Reflect.get against view.constructor.prototype and ArrayBuffer.prototype.{slice,getDetached,getByteLength} via primordials to survive prototype tampering. The C++ versions preserve the same defensive semantics without the JS-side overhead.

benchmark/webstreams/readable-read.js type=bytes (BYOB read path, which exercises these helpers on every chunk) improves by ~15% on my workstation. WPT streams parity preserved: 1403 subtests passing, 0 unexpected failures.

nodejs-github-bot · 2026-05-25T21:10:22Z

Review requested:

@nodejs/gyp

codecov · 2026-05-25T22:35:15Z

Codecov Report

❌ Patch coverage is 80.61224% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.34%. Comparing base (74ccf38) to head (e2c7b65).
⚠️ Report is 85 commits behind head on main.

Files with missing lines	Patch %	Lines
src/node_webstreams.cc	71.21%	6 Missing and 13 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #63570      +/-   ##
==========================================
+ Coverage   90.32%   90.34%   +0.01%     
==========================================
  Files         730      733       +3     
  Lines      234209   236495    +2286     
  Branches    43934    44542     +608     
==========================================
+ Hits       211558   213652    +2094     
- Misses      14372    14549     +177     
- Partials     8279     8294      +15

Files with missing lines	Coverage Δ
lib/internal/webstreams/readablestream.js	`98.54% <100.00%> (+<0.01%)`	⬆️
lib/internal/webstreams/util.js	`99.51% <100.00%> (-0.06%)`	⬇️
src/node_binding.cc	`82.74% <ø> (ø)`
src/node_external_reference.h	`100.00% <ø> (ø)`
src/node_webstreams.cc	`71.21% <71.21%> (ø)`

... and 98 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

addaleax · 2026-05-26T09:43:06Z

+  size_t from_byte_length = BufferByteLength(args[2]);
+
+  bool ok = static_cast<uint64_t>(to_index) + count <= to_byte_length &&
+            static_cast<uint64_t>(from_index) + count <= from_byte_length;


Should this guard against overflows?

defensively, probably. In practice overflow is extremely unlikely so I'd say it's likely optional.

nodejs-github-bot · 2026-05-26T10:16:46Z

CI: https://ci.nodejs.org/job/node-test-pull-request/73731/

Implement three defensive helpers from lib/internal/webstreams/util.js in a new internal binding (src/node_webstreams.cc): * getArrayBufferView * canCopyArrayBuffer * cloneAsUint8Array The previous JavaScript versions used Reflect.get on view.constructor.prototype and called ArrayBuffer.prototype methods through primordials so they would survive prototype tampering. The C++ versions use the V8 ArrayBufferView and ArrayBuffer APIs directly, preserving the same robustness without the JS-side overhead. getArrayBufferView returns { buffer, byteOffset, byteLength } in a single binding crossing, replacing three separate accessors. cloneAsUint8Array uses ArrayBuffer::MaybeNew so the process is not killed on allocation failure. These functions sit on the byte-stream hot paths (ReadableByteStreamController enqueue/read, pull-into descriptor copy, tee clones). ReadableStream type='bytes' throughput on benchmark/webstreams/readable-read.js improves by ~15% on the BYOB read path on my workstation. WPT streams parity is preserved (1403 subtests passing, 0 unexpected failures, identical to baseline). Signed-off-by: Matteo Collina <hello@matteocollina.com>

Renegade334 · 2026-06-02T12:08:58Z

+  CHECK(args[0]->IsArrayBuffer() || args[0]->IsSharedArrayBuffer());
+  CHECK(args[1]->IsUint32());
+  CHECK(args[2]->IsArrayBuffer() || args[2]->IsSharedArrayBuffer());
+  CHECK(args[3]->IsUint32());
+  CHECK(args[4]->IsUint32());


Note that this will crash the process with large buffers, eg.

new ReadableStream({ type: 'bytes', pull(controller) { controller.enqueue(new Uint8Array(2 ** 32)) }, }).getReader({ mode: 'byob' }).read(new Uint8Array(2 ** 32))

The byte lengths should probably be validated as Numbers and read as int64_t with IntegerValue().

jasnell · 2026-06-02T12:27:54Z

+      Number::New(isolate, static_cast<double>(view->ByteLength())),
+  };
+  args.GetReturnValue().Set(
+      Object::New(isolate, Null(isolate), names, values, arraysize(names)));


Using a cached DictionaryTemplate is going to be faster.

Alternatively, returning these as an Array might also be faster.

jasnell · 2026-06-02T12:31:39Z

+// Reflect.get(view.constructor.prototype, ..., view). Uses the V8 API
+// directly so it is immune to prototype tampering and avoids the JS-side
+// overhead of the defensive accessors in lib/internal/.
+void GetArrayBufferView(const FunctionCallbackInfo<Value>& args) {


Given that these aren't specific to web streams, it might make sense to just include in them in the existing utils internal binding rathe than creating Yet Another Internal Binding.

jasnell · 2026-06-02T12:33:35Z

+  Local<Name> names[] = {
+      FIXED_ONE_BYTE_STRING(isolate, "buffer"),
+      FIXED_ONE_BYTE_STRING(isolate, "byteOffset"),
+      FIXED_ONE_BYTE_STRING(isolate, "byteLength"),


If you're not going to use a cached DictionaryTemplate or Array, then these should use the existing env->buffer_string() and add env->byte_offset_string() and env->byte_length_string() to avoid the additional allocations on every call.

jasnell · 2026-06-02T12:34:32Z

+namespace node {
+namespace webstreams {
+
+using v8::ArrayBuffer;
+using v8::ArrayBufferView;
+using v8::Context;
+using v8::FunctionCallbackInfo;
+using v8::Isolate;
+using v8::Local;
+using v8::Name;
+using v8::Null;
+using v8::Number;
+using v8::Object;
+using v8::Uint32;
+using v8::Uint8Array;
+using v8::Value;


Nit: matching convention....

Suggested change

namespace node {

namespace webstreams {

using v8::ArrayBuffer;

using v8::ArrayBufferView;

using v8::Context;

using v8::FunctionCallbackInfo;

using v8::Isolate;

using v8::Local;

using v8::Name;

using v8::Null;

using v8::Number;

using v8::Object;

using v8::Uint32;

using v8::Uint8Array;

using v8::Value;

namespace node {

using v8::ArrayBuffer;

using v8::ArrayBufferView;

using v8::Context;

using v8::FunctionCallbackInfo;

using v8::Isolate;

using v8::Local;

using v8::Name;

using v8::Null;

using v8::Number;

using v8::Object;

using v8::Uint32;

using v8::Uint8Array;

using v8::Value;

namespace webstreams {

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels May 25, 2026

addaleax approved these changes May 26, 2026

View reviewed changes

mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label May 26, 2026

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 26, 2026

Renegade334 reviewed May 26, 2026

View reviewed changes

Comment thread lib/internal/webstreams/util.js Outdated

Renegade334 reviewed May 26, 2026

View reviewed changes

Comment thread src/node_webstreams.cc Outdated

Comment thread src/node_webstreams.cc Outdated

Renegade334 reviewed May 26, 2026

View reviewed changes

Comment thread src/node_webstreams.cc Outdated

anonrig approved these changes May 31, 2026

View reviewed changes

mcollina force-pushed the stream/webstreams-cpp-helpers branch from a71a3f7 to e2c7b65 Compare May 31, 2026 10:38

Renegade334 reviewed Jun 2, 2026

View reviewed changes

jasnell reviewed Jun 2, 2026

View reviewed changes

Uh oh!

Conversation

mcollina commented May 25, 2026

Uh oh!

nodejs-github-bot commented May 25, 2026

Uh oh!

codecov Bot commented May 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented May 26, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

codecov Bot commented May 25, 2026 •

edited

Loading