From 724c69a8cce01aac5f723dc895937c2a16a69f9e Mon Sep 17 00:00:00 2001 From: Orgad Shaneh Date: Sat, 7 Nov 2026 19:38:34 +0200 Subject: [PATCH] src: give small stream reads their own buffer Every read was carved out of the env-wide 64KB StreamReadSlab and handed to JS as a view over it. TLS asks for exactly the decrypted size, so the copy-on-waste path in Commit() never triggers, and any TLS chunk that JS keeps pins the whole slab and every other read in it. Chunks queued for seconds (e.g. audio streamed faster than real time) get the slabs promoted, and their external memory then drives most major GCs. Reads asking for less than a slab now get a right-sized managed buffer, as before the slab existed. Large reads (plain TCP, where libuv suggests 64KB) still use the slab. Assisted-by: Claude Code Signed-off-by: Orgad Shaneh --- src/env.h | 11 +++-- src/stream_base.cc | 6 +++ .../test-tls-small-read-own-buffer.js | 46 +++++++++++++++++++ 3 files changed, 58 insertions(+), 5 deletions(-) create mode 100644 test/parallel/test-tls-small-read-own-buffer.js diff --git a/src/env.h b/src/env.h index 96b0cf52fb29..d52c0d92a329 100644 --- a/src/env.h +++ b/src/env.h @@ -322,11 +322,12 @@ class EnabledDebugList; class StreamReadSlab { public: // Sized to match the read buffer size that libuv suggests for stream - // reads: a slab typically serves a single large read (still avoiding the - // copy that right-sizing the buffer would need), or many small ones. - // Larger slabs amortize allocations further, but stay alive (pinned by - // chunk views) long enough to be promoted to V8's old generation, where - // their external memory is only reclaimed by major GCs. + // reads: a slab serves a single large read (still avoiding the copy that + // right-sizing the buffer would need), or is reused for small reads that + // are copied out of it. Larger slabs amortize allocations further, but + // stay alive (pinned by chunk views) long enough to be promoted to V8's + // old generation, where their external memory is only reclaimed by major + // GCs. static constexpr size_t kSlabSize = 64 * 1024; // Reserve `suggested` bytes. Starts a new slab if the current one does diff --git a/src/stream_base.cc b/src/stream_base.cc index a73a39513fb0..f36a4b7056ba 100644 --- a/src/stream_base.cc +++ b/src/stream_base.cc @@ -725,6 +725,12 @@ void StreamResource::ClearError() { uv_buf_t EmitToJSStreamListener::OnStreamAlloc(size_t suggested_size) { CHECK_NOT_NULL(stream_); Environment* env = static_cast(stream_)->stream_env(); + // Reads that ask for less than a slab (e.g. decrypted TLS records, which + // are allocated at their exact size) get their own buffer: as views over + // a shared slab, any one of them kept by JS would keep the whole slab + // alive, along with every other read that was carved out of it. + if (suggested_size < StreamReadSlab::kSlabSize) + return env->allocate_managed_buffer(suggested_size); return env->stream_read_slab().Allocate(env->isolate(), suggested_size); } diff --git a/test/parallel/test-tls-small-read-own-buffer.js b/test/parallel/test-tls-small-read-own-buffer.js new file mode 100644 index 000000000000..cc65e92f7360 --- /dev/null +++ b/test/parallel/test-tls-small-read-own-buffer.js @@ -0,0 +1,46 @@ +'use strict'; +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); + +// Small TLS reads must not be views over a larger shared buffer: JS keeping +// one of them would keep the whole buffer, and every other read in it, alive. + +const assert = require('assert'); +const tls = require('tls'); +const fixtures = require('../common/fixtures'); + +const kBlock = 320; +const kBlocks = 3; + +const server = tls.createServer({ + key: fixtures.readKey('agent1-key.pem'), + cert: fixtures.readKey('agent1-cert.pem'), +}, common.mustCall((socket) => { + let received = 0; + socket.on('data', common.mustCallAtLeast((chunk) => { + assert.strictEqual(chunk.byteOffset, 0); + assert.strictEqual(chunk.buffer.byteLength, chunk.length); + received += chunk.length; + if (received === kBlocks * kBlock) { + socket.end(); + server.close(); + } else if (received % kBlock === 0) { + // Ask for the next block only once this one has arrived, so that each + // block is a separate small read. + socket.write('n'); + } + })); +})); + +server.listen(0, common.mustCall(() => { + const client = tls.connect({ + port: server.address().port, + rejectUnauthorized: false, + }, common.mustCall(() => { + client.write(Buffer.alloc(kBlock)); + })); + client.on('data', common.mustCall(() => { + client.write(Buffer.alloc(kBlock)); + }, kBlocks - 1)); +}));