From bea785cb9b38d498c0ef2569e63e9f15f653f80d Mon Sep 17 00:00:00 2001 From: GetThatCookie Date: Mon, 3 Aug 2026 23:06:01 +0200 Subject: [PATCH] http2: optimize final response writes Signed-off-by: GetThatCookie --- benchmark/http2/compat.js | 9 ++++- lib/internal/http2/compat.js | 36 +++++++++++++++---- .../test-http2-compat-serverresponse-end.js | 8 +++++ .../test-http2-options-server-response.js | 13 +++++-- 4 files changed, 55 insertions(+), 11 deletions(-) diff --git a/benchmark/http2/compat.js b/benchmark/http2/compat.js index d37bb20c5cdd..597bfb90dd96 100644 --- a/benchmark/http2/compat.js +++ b/benchmark/http2/compat.js @@ -6,6 +6,8 @@ const fs = require('fs'); const file = path.join(path.resolve(__dirname, '../fixtures'), 'alice.html'); const bench = common.createBenchmark(main, { + response: ['end', 'pipe'], + size: [64], requests: [100, 1000, 5000], streams: [1, 10, 20, 40, 100, 200], clients: [2], @@ -13,10 +15,15 @@ const bench = common.createBenchmark(main, { duration: 5, }, { flags: ['--no-warnings'] }); -function main({ requests, streams, clients, duration }) { +function main({ response, size, requests, streams, clients, duration }) { const http2 = require('http2'); + const body = 'a'.repeat(size); const server = http2.createServer(); server.on('request', (req, res) => { + if (response === 'end') { + res.end(body); + return; + } const out = fs.createReadStream(file); res.setHeader('content-type', 'text/html'); out.pipe(res); diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js index d9c123ee846b..1e74ea6a3119 100644 --- a/lib/internal/http2/compat.js +++ b/lib/internal/http2/compat.js @@ -864,17 +864,25 @@ class Http2ServerResponse extends Stream { // skipped entirely when none have been registered. state.finishing = true; - if (chunk !== null && chunk !== undefined) + const hasChunk = chunk !== null && chunk !== undefined; + const endWithChunk = hasChunk && + this.write === Http2ServerResponsePrototypeWrite; + if (endWithChunk) { + if (!stream.headersSent) + this.writeHead(state.statusCode); + } else if (hasChunk) { this.write(chunk, encoding); + } + const previousHeadRequest = state.headRequest; + const previousEnding = state.ending; state.headRequest = stream.headRequest; state.ending = true; + let finishTarget; if (typeof cb === 'function') { - if (stream.writableEnded) - this.once('finish', cb); - else - stream.once('finish', cb); + finishTarget = stream.writableEnded ? this : stream; + finishTarget.once('finish', cb); } if (!stream.headersSent) @@ -882,8 +890,19 @@ class Http2ServerResponse extends Stream { if (this[kState].closed || stream.destroyed) onStreamCloseResponse.call(stream); - else - stream.end(); + else { + try { + if (endWithChunk) + stream.end(chunk, encoding); + else + stream.end(); + } catch (err) { + state.ending = previousEnding; + state.headRequest = previousHeadRequest; + finishTarget?.removeListener('finish', cb); + throw err; + } + } return this; } @@ -998,6 +1017,9 @@ class Http2ServerResponse extends Stream { } } +// Preserve user-defined write behavior when end() receives a final chunk. +const Http2ServerResponsePrototypeWrite = Http2ServerResponse.prototype.write; + function onServerStream(ServerRequest, ServerResponse, stream, headers, flags, rawHeaders) { const server = this; diff --git a/test/parallel/test-http2-compat-serverresponse-end.js b/test/parallel/test-http2-compat-serverresponse-end.js index 03c3db2c6e88..874c3ebf84f8 100644 --- a/test/parallel/test-http2-compat-serverresponse-end.js +++ b/test/parallel/test-http2-compat-serverresponse-end.js @@ -24,6 +24,14 @@ const { // It may be invoked repeatedly without throwing errors // but callback will only be called once const server = createServer(mustCall((request, response) => { + const stream = response.stream; + const streamEnd = stream.end; + stream.write = mustNotCall(); + stream.end = mustCall(function(chunk, encoding) { + assert.strictEqual(chunk, 'end'); + assert.strictEqual(encoding, 'utf8'); + return streamEnd.call(this, chunk, encoding); + }); response.end('end', 'utf8', mustCall(() => { response.end(mustCall()); process.nextTick(() => { diff --git a/test/parallel/test-http2-options-server-response.js b/test/parallel/test-http2-options-server-response.js index 6f1ae1881d22..3dc3175934a1 100644 --- a/test/parallel/test-http2-options-server-response.js +++ b/test/parallel/test-http2-options-server-response.js @@ -3,20 +3,27 @@ const common = require('../common'); if (!common.hasCrypto) common.skip('missing crypto'); +const assert = require('assert'); const h2 = require('http2'); class MyServerResponse extends h2.Http2ServerResponse { status(code) { return this.writeHead(code, { 'Content-Type': 'text/plain' }); } + + write(...args) { + this.writeCalled = true; + return super.write(...args); + } } const server = h2.createServer({ Http2ServerResponse: MyServerResponse -}, (req, res) => { +}, common.mustCall((req, res) => { res.status(200); - res.end(); -}); + res.end('body'); + assert.strictEqual(res.writeCalled, true); +})); server.listen(0); server.on('listening', common.mustCall(() => {