nodejs / help

:sparkles: Need help with Node.js? File an Issue here. :rocket:
1.46k stars 279 forks source link

WebSocket Data Transfer Slows Down After Node.js 16.14.2 Release #4464

Open harendrasp opened 1 month ago

harendrasp commented 1 month ago

Node.js Version

18.20.4

NPM Version

10.7.0

Operating System

ubuntu 22.04

Subsystem

buffer, events, process, stream, v8

Description

Data transfer of 4K frames (3840 2160 3) via WebSocket has become significantly slower in Node.js 18.20.4. I set up a WebSocket server, and from the browser, established a connection to transfer 4K frames. When comparing the performance of data transfer between Node.js 16.14.2 and 18.20.4, there is a noticeable degradation in transfer speed. I'm not certain where the issue lies within the Node.js subsystems, so I've tagged multiple areas that I believe could be relevant.

Minimal Reproduction

Steps to Simulate the Issue:

  1. Install Node.js versions 16.14.2 and 18.20.4 for comparison (both are LTS versions).
  2. Run npm init to initialize the project.
  3. Install the WebSocket library using npm i websocket.
  4. Copy the attached server.js file into the current directory.
  5. Start the server with node server.js.
  6. Open a browser, navigate to the console.
  7. Paste and execute the client.js code.
  8. Output: Compare the Round-Trip Time (RTT) between Node.js 16.14.2 and 18.20.4. Please note that same websocket version is used for both node js versions. Archive 2.zip

Output

Output Comparison:

Node 16.14.2: Round-trip time: 220 ms Node 18.20.4: Round-trip time: 400 ms This indicates that Node.js 16 has a better WebSocket transfer speed compared to Node.js 18. The slowdown is consistent from Node.js 17.2.0 through Node.js 18.20.4. Although the performance improves in Node.js 19 and Node.js 20, there is still a noticeable degradation. Specifically, Node.js 19 onwards is approximately 20% slower than Node.js 16, while Node.js 18 is 100% slower compared to Node.js 16.

Before You Submit

RedYetiDev commented 1 month ago

Hi! Could you host a minimal reproduction script in a GitHub repository, rather than providing an archive?

harendrasp commented 1 month ago

Steps to simulate the issue: (GitHub repository: nodetesting)

  1. Open the terminal and create a directory, mkdir -p nodetesting
  2. Run cd nodetesting
  3. Run npm init -y
  4. Run npm i ws
  5. Save the below code in a file, e.g., server.js.
  6. Run node server.js
    
    /**
    * server.js code
    * Import necessary modules
    */
    const WebSocket = require('ws');
    const wsPort = 8057;

// Create a WebSocket server const server = new WebSocket.Server({ port: wsPort });

// Handle WebSocket connections server.on('connection', (ws) => { console.log('Client connected');

ws.on('message', (message) => { ws.send(message); // Echo back the received message });

ws.on('close', () => { console.log('Client disconnected'); }); });

// Log WebSocket server status console.log(WebSocket server running on ws://localhost:${wsPort});


6. Open the chrome browser(with www.google.com), go to console.
7. Paste and execute the below code.

const ws = new WebSocket('ws://localhost:8057');

const messageSize = 3840 2160 3; // 4K RGB frame data. let startTime; let roundTripTimes = []; const totalPackets = 20;

ws.onopen = () => { console.log('Connected to server'); startTime = performance.now(); ws.send(new ArrayBuffer(messageSize)); };

ws.onmessage = (event) => { const endTime = performance.now(); const duration = endTime - startTime; roundTripTimes.push(duration); console.log(Round-trip time: ${duration.toFixed(3)}ms);

if (roundTripTimes.length < totalPackets) {
    startTime = performance.now();
    ws.send(new ArrayBuffer(messageSize));
} else {
    const averageTime = roundTripTimes.reduce((a, b) => a + b, 0) / roundTripTimes.length;
    console.log(`Average round-trip time over ${roundTripTimes.length} messages: ${averageTime.toFixed(3)}ms`);
    ws.close();
}

};

ws.onclose = () => { console.log('Connection closed'); };

ws.onerror = (error) => { console.error('WebSocket error:', error); };


8. Observe the Round-trip time and compare for node 16 and node 18.
harendrasp commented 1 month ago

@RedYetiDev ^ I hope these steps will help simulate the performance issue in WebSocket data transfer.

harendrasp commented 1 month ago

@RedYetiDev Created a git repo with the above steps: https://github.com/harendrasp/nodetesting/tree/main

harendrasp commented 1 month ago
node 16 node 18
RedYetiDev commented 1 month ago

Can you reproduce using the native websocket, and not the the WS library?

Additionally, can you reproduce without Google chrome, having the client and server in the same file?

harendrasp commented 1 month ago

@RedYetiDev Local Setup (Client and WebSocket Server on the Same Machine): Node 20: Average round-trip time over 20 messages: 123.096 ms Node 18: Average round-trip time over 20 messages: 86.739 ms Node 16: Average round-trip time over 20 messages: 85.541 ms

Browser as Client, Local WebSocket Server: Node 20: Average round-trip time over 20 messages: 276.230 ms Node 18: Average round-trip time over 20 messages: 405.320 ms Node 16: Average round-trip time over 20 messages: 263.475 ms

Conclusion: Local Client: Node 16 ≈ Node 18 > Node 20 Browser Client: Node 16 ≈ Node 20 > Node 18

Additionally, I've observed that the browser as client performance has been slower since the release of Node.js 17.2.0 through Node.js 18.20.4.

harendrasp commented 1 month ago

@RedYetiDev Does Node.js provide a native WebSocket implementation, or do I need to create one using the http module? For your reference, I used the same WebSocket library (version 8.18.0) across all the scenarios mentioned above.

RedYetiDev commented 1 month ago

Node.js provides a native WebSocket. Depending on the version, it may need to be enabled with --experimental-websocket.

harendrasp commented 1 month ago

@RedYetiDev Yes, there was a regression in versions 17 and 18 that was addressed in Node 20. However, Node 16 still outperforms Node 20 in long-duration data transfers. For example, if each frame has a delay of 10ms and 1,000 frames are transferred, the total delay accumulates to 10 seconds. Do you have any suggestions to mitigate this delay in Node 18 or Node 20? I need to upgrade to a newer Node version, but the performance degradation is a concern.

RedYetiDev commented 1 month ago

If it's reproducible in the native websocket implementation, it has to do with the undici version present.

harendrasp commented 1 month ago

In native WebSocket, the issue is reproducible starting from Node.js 20, whereas Node.js 18 performs similarly to Node.js 16

harendrasp commented 1 month ago

In the browser, the issue is reproducible starting from Node.js 17, but it has improved in Node.js 20.

harendrasp commented 1 month ago

@RedYetiDev Are there any workarounds to improve performance in Node.js 18, such as configuration changes or downgrading the undici library?

RedYetiDev commented 1 month ago

I'm not sure about workarounds, but @nodejs/undici might know.

harendrasp commented 1 month ago

@RedYetiDev I've reported a bug for undici. I see the undici library was introduced in Node.js 18 however, the slowness issue actually originates from Node.js 17.2.0.

KhafraDev commented 1 month ago

cc @lpinca

RedYetiDev commented 1 month ago

For reference, the undici issue is https://github.com/nodejs/undici/issues/3455

lpinca commented 1 month ago

It has nothing to do with undici if the issue can be reproduces without its WebSocket implementation. I think the difference is caused by different V8 and libuv versions (I guess the ws version is the same). Also, it seems that Node.js 22.6.0 produces results similar to Node.js 16.20.2. I'm not sure if there is anything actionable to do.

harendrasp commented 1 month ago

@lpinca Agree node 22 is giving similar performance as 16. However, Upgrading directly to Node.js 22 may lead to compatibility issues, as some libraries and dependencies might not yet fully support this version. For example, tools like Microsoft/Rush may not be compatible with Node.js 22 at the time of your upgrade. Since it can be challenging to ensure all dependent libraries are updated to match the latest Node.js version, it is advisable to consider upgrading to Node.js 18 or 20 first.

mcollina commented 4 weeks ago

I don't think there is much to do for us. If the "fix" for this was backportable, it would have been packported already/or it is incoming to v20 in the latest release. If it's a semver-major change (like a V8 update), it might be impossible to backport. Backporting individual V8 commits is possible, but unless locating the correct one would be incredibly hard.

In other terms, I don't think there is a volunteer for doing all this work right now, considering that v22 has it fixed already.