From cd6f370c2855ff0d4ed99030a5a4494323858cb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Wed, 28 Sep 2022 20:03:55 +0200 Subject: [PATCH 1/3] unbuffered delivery for userInput response only --- demo/server.js | 35 ++++++++++++++++++++++++++++------- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/demo/server.js b/demo/server.js index 8bb684a2..c594da3e 100644 --- a/demo/server.js +++ b/demo/server.js @@ -79,14 +79,25 @@ function startServer() { console.log('Connected to terminal ' + term.pid); ws.send(logs[term.pid]); + // unbuffered delivery after user input + let userInput = false; + // string message buffering - function buffer(socket, timeout) { + function buffer(socket, timeout, maxSize) { let s = ''; let sender = null; return (data) => { s += data; - if (!sender) { - sender = queueMicrotask(() => { + if (s.length > maxSize || userInput) { + userInput = false; + socket.send(s); + s = ''; + if (sender) { + clearTimeout(sender); + sender = null; + } + } else if (!sender) { + sender = setTimeout(() => { socket.send(s); s = ''; sender = null; @@ -95,15 +106,24 @@ function startServer() { }; } // binary message buffering - function bufferUtf8(socket, timeout) { + function bufferUtf8(socket, timeout, maxSize) { let buffer = []; let sender = null; let length = 0; return (data) => { buffer.push(data); length += data.length; - if (!sender) { - sender = queueMicrotask(() => { + if (length > maxSize || userInput) { + userInput = false; + socket.send(Buffer.concat(buffer, length)); + buffer = []; + length = 0; + if (sender) { + clearTimeout(sender); + sender = null; + } + } else if (!sender) { + sender = setTimeout(() => { socket.send(Buffer.concat(buffer, length)); buffer = []; sender = null; @@ -112,7 +132,7 @@ function startServer() { } }; } - const send = USE_BINARY ? bufferUtf8(ws, 5) : buffer(ws, 5); + const send = (USE_BINARY ? bufferUtf8 : buffer)(ws, 2, 262144); // WARNING: This is a naive implementation that will not throttle the flow of data. This means // it could flood the communication channel and make the terminal unresponsive. Learn more about @@ -126,6 +146,7 @@ function startServer() { }); ws.on('message', function(msg) { term.write(msg); + userInput = true; }); ws.on('close', function () { term.kill(); From 193d305dcd37537aa47758e580059ffd220b8723 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Thu, 29 Sep 2022 10:36:08 +0200 Subject: [PATCH 2/3] revert timeout to 5ms, remove log in server.js --- demo/server.js | 17 +++-------------- src/common/input/WriteBuffer.ts | 2 +- 2 files changed, 4 insertions(+), 15 deletions(-) diff --git a/demo/server.js b/demo/server.js index c594da3e..0e82f9e9 100644 --- a/demo/server.js +++ b/demo/server.js @@ -16,8 +16,7 @@ function startServer() { var app = express(); expressWs(app); - var terminals = {}, - logs = {}; + var terminals = {}; app.use('/xterm.css', express.static(__dirname + '/../css/xterm.css')); app.get('/logo.png', (req, res) => { @@ -55,10 +54,6 @@ function startServer() { console.log('Created terminal with PID: ' + term.pid); terminals[term.pid] = term; - logs[term.pid] = ''; - term.on('data', function(data) { - logs[term.pid] += data; - }); res.send(term.pid.toString()); res.end(); }); @@ -77,7 +72,6 @@ function startServer() { app.ws('/terminals/:pid', function (ws, req) { var term = terminals[parseInt(req.params.pid)]; console.log('Connected to terminal ' + term.pid); - ws.send(logs[term.pid]); // unbuffered delivery after user input let userInput = false; @@ -132,17 +126,13 @@ function startServer() { } }; } - const send = (USE_BINARY ? bufferUtf8 : buffer)(ws, 2, 262144); + const send = (USE_BINARY ? bufferUtf8 : buffer)(ws, 5, 262144); // WARNING: This is a naive implementation that will not throttle the flow of data. This means // it could flood the communication channel and make the terminal unresponsive. Learn more about // the problem and how to implement flow control at https://xtermjs.org/docs/guides/flowcontrol/ term.on('data', function(data) { - try { - send(data); - } catch (ex) { - // The WebSocket is not open, ignore - } + send(data); }); ws.on('message', function(msg) { term.write(msg); @@ -153,7 +143,6 @@ function startServer() { console.log('Closed terminal ' + term.pid); // Clean things up delete terminals[term.pid]; - delete logs[term.pid]; }); }); diff --git a/src/common/input/WriteBuffer.ts b/src/common/input/WriteBuffer.ts index 4f316f24..8cb1edf4 100644 --- a/src/common/input/WriteBuffer.ts +++ b/src/common/input/WriteBuffer.ts @@ -106,7 +106,7 @@ export class WriteBuffer { this._bufferOffset = 0; // If this is the first write call after the user has done some input, - // parse it immediately in an upcoming microtask to minimize reduce input, + // parse it immediately to minimize reduce input, // otherwise schedule for the next event if (this._didUserInput) { this._didUserInput = false; From 150499b7e5ee56fa4bc0985f0099ce739ff6f802 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Thu, 29 Sep 2022 10:46:25 +0200 Subject: [PATCH 3/3] comment fix --- src/common/input/WriteBuffer.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/input/WriteBuffer.ts b/src/common/input/WriteBuffer.ts index 8cb1edf4..5e19105d 100644 --- a/src/common/input/WriteBuffer.ts +++ b/src/common/input/WriteBuffer.ts @@ -106,7 +106,7 @@ export class WriteBuffer { this._bufferOffset = 0; // If this is the first write call after the user has done some input, - // parse it immediately to minimize reduce input, + // parse it immediately to minimize input latency, // otherwise schedule for the next event if (this._didUserInput) { this._didUserInput = false;