From 0ccddec23b0849f3ff985677fcd32fde03a2edcb Mon Sep 17 00:00:00 2001 From: Claude Code Date: Mon, 6 Oct 2025 17:28:28 +0000 Subject: [PATCH 1/2] Address Gemini code review comments on PR #494 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit High Priority: - Separate setup logic from measured operations in connection benchmarks - Create shared connection once, reuse across all benchmark iterations - Dramatically improved benchmark accuracy (2.4M ops/sec for ping vs 33K before) Medium Priority: - Lazily initialize suite results in track-performance.mjs to prevent empty entries - Remove redundant test.include from vitest.bench.config.mjs - Add bench:compare script to package.json to match README documentation Updated baseline.json with new accurate performance measurements: - Connection operations now measure actual send performance (not setup) - Frame serialization remains consistent at 4M+ ops/sec - Ping/pong operations: 2.4M / 1.9M ops/sec (previously 33K) - Message sending: 900K / 220K / 108K ops/sec (previously 25-28K) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- package.json | 1 + test/benchmark/baseline.json | 30 ++++++------- .../benchmark/connection-operations.bench.mjs | 43 +++++++------------ test/benchmark/track-performance.mjs | 6 ++- vitest.bench.config.mjs | 1 - 5 files changed, 36 insertions(+), 45 deletions(-) diff --git a/package.json b/package.json index fcdbd98e..c550e20f 100644 --- a/package.json +++ b/package.json @@ -57,6 +57,7 @@ "test:autobahn": "cd test/autobahn && ./run-wstest.js", "bench": "vitest bench --run --config vitest.bench.config.mjs", "bench:baseline": "node test/benchmark/track-performance.mjs save", + "bench:compare": "node test/benchmark/track-performance.mjs compare", "bench:check": "node test/benchmark/track-performance.mjs check", "lint": "eslint lib/**/*.js test/**/*.js", "lint:fix": "eslint lib/**/*.js test/**/*.js --fix" diff --git a/test/benchmark/baseline.json b/test/benchmark/baseline.json index bcbd72d1..1da79efb 100644 --- a/test/benchmark/baseline.json +++ b/test/benchmark/baseline.json @@ -1,21 +1,19 @@ { - "timestamp": "2025-10-06T16:43:17.731Z", + "timestamp": "2025-10-06T17:27:59.641Z", "results": { - "WebSocketConnection Performance 3866ms": { - "create connection instance": 28323.39, - "send small UTF-8 message": 28201.35, - "send medium UTF-8 message (1KB)": 24615.97, - "send binary message (1KB)": 24889.31, - "send ping frame": 31562.37, - "send pong frame": 32393.72 + "WebSocketConnection Performance 7359ms": { + "create connection instance": 28847.63, + "send small UTF-8 message": 914160.4, + "send medium UTF-8 message (1KB)": 108086.21, + "send binary message (1KB)": 222918.81, + "send ping frame": 2375454.67, + "send pong frame": 1935975.19 }, - "WebSocketFrame Performance 10001ms": { - "serialize small text frame (17 bytes, unmasked)": 4427042.23, - "serialize small text frame (17 bytes, masked)": 3005215.87, - "serialize medium binary frame (1KB)": 4239270.36, - "serialize large binary frame (64KB)": 4024552.34 - }, - "WebSocketConnection Performance": {}, - "WebSocketFrame Performance": {} + "WebSocketFrame Performance 9745ms": { + "serialize small text frame (17 bytes, unmasked)": 4240401.67, + "serialize small text frame (17 bytes, masked)": 3070532.44, + "serialize medium binary frame (1KB)": 4418217.61, + "serialize large binary frame (64KB)": 3918678.38 + } } } \ No newline at end of file diff --git a/test/benchmark/connection-operations.bench.mjs b/test/benchmark/connection-operations.bench.mjs index dc15c13b..88197881 100644 --- a/test/benchmark/connection-operations.bench.mjs +++ b/test/benchmark/connection-operations.bench.mjs @@ -3,6 +3,17 @@ import WebSocketConnection from '../../lib/WebSocketConnection.js'; import { MockSocket } from '../helpers/mocks.mjs'; describe('WebSocketConnection Performance', () => { + // Pre-allocate messages and buffers outside benchmarks + const smallMessage = 'Hello, WebSocket!'; + const mediumMessage = 'x'.repeat(1024); + const binaryBuffer = Buffer.alloc(1024); + + // Pre-create connection for send operations (created once, reused across all iterations) + const sharedSocket = new MockSocket(); + const sharedConnection = new WebSocketConnection(sharedSocket, [], 'echo-protocol', false, {}); + sharedConnection._addSocketEventListeners(); + sharedConnection.state = 'open'; + bench('create connection instance', () => { const socket = new MockSocket(); const connection = new WebSocketConnection(socket, [], 'echo-protocol', false, {}); @@ -10,44 +21,22 @@ describe('WebSocketConnection Performance', () => { }); bench('send small UTF-8 message', () => { - const socket = new MockSocket(); - const connection = new WebSocketConnection(socket, [], 'echo-protocol', false, {}); - connection._addSocketEventListeners(); - connection.state = 'open'; - connection.sendUTF('Hello, WebSocket!'); + sharedConnection.sendUTF(smallMessage); }); bench('send medium UTF-8 message (1KB)', () => { - const socket = new MockSocket(); - const connection = new WebSocketConnection(socket, [], 'echo-protocol', false, {}); - connection._addSocketEventListeners(); - connection.state = 'open'; - const message = 'x'.repeat(1024); - connection.sendUTF(message); + sharedConnection.sendUTF(mediumMessage); }); bench('send binary message (1KB)', () => { - const socket = new MockSocket(); - const connection = new WebSocketConnection(socket, [], 'echo-protocol', false, {}); - connection._addSocketEventListeners(); - connection.state = 'open'; - const buffer = Buffer.alloc(1024); - connection.sendBytes(buffer); + sharedConnection.sendBytes(binaryBuffer); }); bench('send ping frame', () => { - const socket = new MockSocket(); - const connection = new WebSocketConnection(socket, [], 'echo-protocol', false, {}); - connection._addSocketEventListeners(); - connection.state = 'open'; - connection.ping(); + sharedConnection.ping(); }); bench('send pong frame', () => { - const socket = new MockSocket(); - const connection = new WebSocketConnection(socket, [], 'echo-protocol', false, {}); - connection._addSocketEventListeners(); - connection.state = 'open'; - connection.pong(); + sharedConnection.pong(); }); }); diff --git a/test/benchmark/track-performance.mjs b/test/benchmark/track-performance.mjs index ba959b19..469a6675 100755 --- a/test/benchmark/track-performance.mjs +++ b/test/benchmark/track-performance.mjs @@ -35,13 +35,17 @@ function parseBenchmarkOutput(output) { // Detect suite name if (line.includes('> WebSocket')) { currentSuite = line.match(/> (.*)/)[1].trim(); - results[currentSuite] = {}; + // Don't initialize suite here - wait until first benchmark is found } // Parse benchmark results const benchMatch = line.match(/^\s*[·•]\s+(.+?)\s+(\d+(?:,\d+)*(?:\.\d+)?)\s/); if (benchMatch && currentSuite) { const [, name, hz] = benchMatch; + // Lazily initialize suite only when first benchmark is found + if (!results[currentSuite]) { + results[currentSuite] = {}; + } results[currentSuite][name.trim()] = parseFloat(hz.replace(/,/g, '')); } } diff --git a/vitest.bench.config.mjs b/vitest.bench.config.mjs index 00d879a8..b10803ec 100644 --- a/vitest.bench.config.mjs +++ b/vitest.bench.config.mjs @@ -2,7 +2,6 @@ import { defineConfig } from 'vitest/config'; export default defineConfig({ test: { - include: ['test/benchmark/**/*.bench.mjs'], benchmark: { include: ['test/benchmark/**/*.bench.mjs'], exclude: ['node_modules/', 'test/unit/', 'test/integration/'], From afd2096f66e39f3aa756bc78e2e7764b7b0d02b2 Mon Sep 17 00:00:00 2001 From: Claude Code Date: Mon, 6 Oct 2025 17:33:32 +0000 Subject: [PATCH 2/2] Add explanation for not using beforeAll in benchmarks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Vitest's benchmark runner doesn't execute hooks (beforeAll/beforeEach) before benchmarks in the same way as test(). Direct initialization at module scope ensures the shared connection is available when benchmarks run. Attempted using beforeAll() but it resulted in 0 hz for all benchmarks using sharedConnection, indicating the hook wasn't executed before benchmark iterations. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- test/benchmark/connection-operations.bench.mjs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/benchmark/connection-operations.bench.mjs b/test/benchmark/connection-operations.bench.mjs index 88197881..efe48a7f 100644 --- a/test/benchmark/connection-operations.bench.mjs +++ b/test/benchmark/connection-operations.bench.mjs @@ -8,7 +8,9 @@ describe('WebSocketConnection Performance', () => { const mediumMessage = 'x'.repeat(1024); const binaryBuffer = Buffer.alloc(1024); - // Pre-create connection for send operations (created once, reused across all iterations) + // Shared connection for send operations (created once, reused across all iterations) + // Note: We initialize this directly rather than using beforeAll() because Vitest's + // benchmark runner doesn't execute hooks before benchmarks in the same way as test() const sharedSocket = new MockSocket(); const sharedConnection = new WebSocketConnection(sharedSocket, [], 'echo-protocol', false, {}); sharedConnection._addSocketEventListeners();