perf(terminal): flush shell output on the event-loop turn instead of a fixed 8ms timer (#1085)
* perf(terminal): flush shell output on the event-loop turn, not a fixed 8ms timer SSH/PTY output was coalesced and shipped to the renderer on a fixed 8ms timer. For interactive use that interval is pure added latency: every echoed keystroke waits out the timer before it can paint, so typing feels slightly behind. Replace the timer with turn-based (setImmediate) coalescing in a single shared ptyOutputBuffer module, used by the SSH, local, telnet, and mosh paths. A single echoed keystroke is now forwarded almost immediately, while data arriving in the same turn still collapses into one IPC send, and a 16KB size cap still forces an immediate flush under heavy output. Also de-duplicates two copies of the buffering logic (SSH had an inline copy; local/telnet/mosh shared another) and adds unit tests for the buffer. Related to #1084. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(terminal): drop orphaned flushTimeout reference in SSH close handler The SSH stream "close" handler still cleared `flushTimeout`, a variable that lived in the inline buffer removed when this path moved to the shared ptyOutputBuffer. Reading it now throws ReferenceError on every channel close, aborting the cleanup and exit signaling. The shared buffer's flush() cancels any pending flush internally, so the timer bookkeeping is removed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
63
electron/bridges/ptyOutputBuffer.cjs
Normal file
63
electron/bridges/ptyOutputBuffer.cjs
Normal file
@@ -0,0 +1,63 @@
|
||||
"use strict";
|
||||
|
||||
/**
|
||||
* Coalescing output buffer for terminal/PTY data on its way to the renderer.
|
||||
*
|
||||
* Incoming shell data is accumulated and delivered to `sendFn` in batches to
|
||||
* keep IPC traffic down, but the batch is flushed on the *next event-loop turn*
|
||||
* (`setImmediate`) rather than after a fixed time interval. A fixed interval
|
||||
* adds that whole interval as latency to interactive echo — every keystroke
|
||||
* round-trips through the buffer and waits out the timer before it can paint.
|
||||
* Turn-based flushing coalesces only the data that has already arrived in the
|
||||
* current turn, so a single echoed keystroke is forwarded almost immediately
|
||||
* while bursts of output still collapse into one send.
|
||||
*
|
||||
* A byte cap still forces an immediate, synchronous flush so a flood of output
|
||||
* can't grow the buffer without bound between turns.
|
||||
*
|
||||
* @param {(data: string) => void} sendFn delivers an accumulated batch
|
||||
* @param {{ maxBufferSize?: number }} [options]
|
||||
* @returns {{ bufferData: (data: string) => void, flush: () => void }}
|
||||
*/
|
||||
function createPtyOutputBuffer(sendFn, options = {}) {
|
||||
const maxBufferSize = options.maxBufferSize ?? 16384; // 16KB
|
||||
|
||||
let dataBuffer = "";
|
||||
let scheduled = null;
|
||||
|
||||
const cancelScheduled = () => {
|
||||
if (scheduled) {
|
||||
clearImmediate(scheduled);
|
||||
scheduled = null;
|
||||
}
|
||||
};
|
||||
|
||||
const flushNow = () => {
|
||||
scheduled = null;
|
||||
if (dataBuffer.length > 0) {
|
||||
const pending = dataBuffer;
|
||||
dataBuffer = "";
|
||||
sendFn(pending);
|
||||
}
|
||||
};
|
||||
|
||||
const bufferData = (data) => {
|
||||
dataBuffer += data;
|
||||
if (dataBuffer.length >= maxBufferSize) {
|
||||
// Large enough to ship right now — don't wait for the turn flush.
|
||||
cancelScheduled();
|
||||
flushNow();
|
||||
} else if (!scheduled) {
|
||||
scheduled = setImmediate(flushNow);
|
||||
}
|
||||
};
|
||||
|
||||
const flush = () => {
|
||||
cancelScheduled();
|
||||
flushNow();
|
||||
};
|
||||
|
||||
return { bufferData, flush };
|
||||
}
|
||||
|
||||
module.exports = { createPtyOutputBuffer };
|
||||
90
electron/bridges/ptyOutputBuffer.test.cjs
Normal file
90
electron/bridges/ptyOutputBuffer.test.cjs
Normal file
@@ -0,0 +1,90 @@
|
||||
const test = require("node:test");
|
||||
const assert = require("node:assert/strict");
|
||||
|
||||
const { createPtyOutputBuffer } = require("./ptyOutputBuffer.cjs");
|
||||
|
||||
/** Resolve after one event-loop turn (immediates have run). */
|
||||
const tick = () => new Promise((resolve) => setImmediate(resolve));
|
||||
|
||||
test("coalesces data buffered within the same turn into a single send", async () => {
|
||||
const sends = [];
|
||||
const buffer = createPtyOutputBuffer((data) => sends.push(data));
|
||||
|
||||
buffer.bufferData("a");
|
||||
buffer.bufferData("b");
|
||||
buffer.bufferData("c");
|
||||
|
||||
// Nothing is sent synchronously while still in the same turn.
|
||||
assert.equal(sends.length, 0);
|
||||
|
||||
await tick();
|
||||
|
||||
assert.deepEqual(sends, ["abc"]);
|
||||
});
|
||||
|
||||
test("flushes within a single event-loop turn (not on a fixed delay)", async () => {
|
||||
const sends = [];
|
||||
const buffer = createPtyOutputBuffer((data) => sends.push(data));
|
||||
|
||||
buffer.bufferData("x");
|
||||
|
||||
// A fixed-interval (e.g. 8ms) buffer would NOT have flushed after one
|
||||
// immediate turn. Turn-based flushing must have delivered it by now.
|
||||
await tick();
|
||||
|
||||
assert.deepEqual(sends, ["x"]);
|
||||
});
|
||||
|
||||
test("flushes immediately and synchronously once the size cap is reached", async () => {
|
||||
const sends = [];
|
||||
const buffer = createPtyOutputBuffer((data) => sends.push(data), {
|
||||
maxBufferSize: 4,
|
||||
});
|
||||
|
||||
buffer.bufferData("ab");
|
||||
assert.equal(sends.length, 0); // under cap, still pending
|
||||
|
||||
buffer.bufferData("cd"); // now "abcd" hits the 4-byte cap
|
||||
|
||||
// Cap flush happens synchronously, without waiting for the turn.
|
||||
assert.deepEqual(sends, ["abcd"]);
|
||||
|
||||
// The pending turn flush must have been cancelled — no empty/duplicate send.
|
||||
await tick();
|
||||
assert.deepEqual(sends, ["abcd"]);
|
||||
});
|
||||
|
||||
test("flush() forces a synchronous send and cancels the pending turn", async () => {
|
||||
const sends = [];
|
||||
const buffer = createPtyOutputBuffer((data) => sends.push(data));
|
||||
|
||||
buffer.bufferData("hello");
|
||||
buffer.flush();
|
||||
|
||||
assert.deepEqual(sends, ["hello"]);
|
||||
|
||||
await tick();
|
||||
assert.deepEqual(sends, ["hello"]); // not sent twice
|
||||
});
|
||||
|
||||
test("flush() with an empty buffer does not send", async () => {
|
||||
const sends = [];
|
||||
const buffer = createPtyOutputBuffer((data) => sends.push(data));
|
||||
|
||||
buffer.flush();
|
||||
|
||||
assert.equal(sends.length, 0);
|
||||
});
|
||||
|
||||
test("keeps batching after a flush", async () => {
|
||||
const sends = [];
|
||||
const buffer = createPtyOutputBuffer((data) => sends.push(data));
|
||||
|
||||
buffer.bufferData("first");
|
||||
await tick();
|
||||
|
||||
buffer.bufferData("second");
|
||||
await tick();
|
||||
|
||||
assert.deepEqual(sends, ["first", "second"]);
|
||||
});
|
||||
@@ -17,6 +17,7 @@ const passphraseHandler = require("./passphraseHandler.cjs");
|
||||
const hostKeyVerifier = require("./hostKeyVerifier.cjs");
|
||||
const { createProxySocket } = require("./proxyUtils.cjs");
|
||||
const { attachX11Forwarding } = require("./x11Forwarding.cjs");
|
||||
const { createPtyOutputBuffer } = require("./ptyOutputBuffer.cjs");
|
||||
const {
|
||||
buildAuthHandler,
|
||||
createKeyboardInteractiveHandler,
|
||||
@@ -1287,35 +1288,14 @@ async function startSSHSession(event, options) {
|
||||
});
|
||||
}
|
||||
|
||||
// Data buffering for reduced IPC overhead
|
||||
let dataBuffer = '';
|
||||
let flushTimeout = null;
|
||||
const FLUSH_INTERVAL = 8; // ms - flush every 8ms for ~120fps equivalent
|
||||
const MAX_BUFFER_SIZE = 16384; // 16KB - flush immediately if buffer gets too large
|
||||
|
||||
const flushBuffer = () => {
|
||||
if (dataBuffer.length > 0) {
|
||||
const contents = event.sender;
|
||||
safeSend(contents, "netcatty:data", { sessionId, data: dataBuffer });
|
||||
dataBuffer = '';
|
||||
}
|
||||
flushTimeout = null;
|
||||
};
|
||||
|
||||
const bufferData = (data) => {
|
||||
dataBuffer += data;
|
||||
// Immediate flush for large chunks
|
||||
if (dataBuffer.length >= MAX_BUFFER_SIZE) {
|
||||
if (flushTimeout) {
|
||||
clearTimeout(flushTimeout);
|
||||
flushTimeout = null;
|
||||
}
|
||||
flushBuffer();
|
||||
} else if (!flushTimeout) {
|
||||
// Schedule flush
|
||||
flushTimeout = setTimeout(flushBuffer, FLUSH_INTERVAL);
|
||||
}
|
||||
};
|
||||
// Coalesce shell output and deliver it to the renderer on the next
|
||||
// event-loop turn (see ptyOutputBuffer) rather than on a fixed timer,
|
||||
// so interactive echo isn't held back by the batch interval. A size
|
||||
// cap still forces an immediate flush for bursts of output.
|
||||
const { bufferData, flush: flushBuffer } = createPtyOutputBuffer((data) => {
|
||||
const contents = event.sender;
|
||||
safeSend(contents, "netcatty:data", { sessionId, data });
|
||||
});
|
||||
|
||||
const sshZmodemSentry = createZmodemSentry({
|
||||
sessionId,
|
||||
@@ -1392,10 +1372,8 @@ async function startSSHSession(event, options) {
|
||||
});
|
||||
|
||||
stream.on("close", () => {
|
||||
// Always flush buffered data regardless of session state
|
||||
if (flushTimeout) {
|
||||
clearTimeout(flushTimeout);
|
||||
}
|
||||
// Always flush buffered data regardless of session state.
|
||||
// flushBuffer() cancels any pending scheduled flush internally.
|
||||
flushBuffer();
|
||||
sessionLogStreamManager.stopStream(sessionId, logStreamToken);
|
||||
if (detachX11Forwarding) {
|
||||
|
||||
@@ -25,6 +25,7 @@ const moshHandshake = require("./moshHandshake.cjs");
|
||||
const tempDirBridge = require("./tempDirBridge.cjs");
|
||||
const { createTelnetAutoLogin } = require("./telnetAutoLogin.cjs");
|
||||
const telnetProtocol = require("./telnetProtocol.cjs");
|
||||
const { createPtyOutputBuffer } = require("./ptyOutputBuffer.cjs");
|
||||
|
||||
const execFileAsync = promisify(execFile);
|
||||
|
||||
@@ -80,51 +81,6 @@ function init(deps) {
|
||||
electronModule = deps.electronModule;
|
||||
}
|
||||
|
||||
/**
|
||||
* Create an 8ms/16KB PTY data buffer for reduced IPC overhead.
|
||||
* Mirrors the SSH stream buffering strategy in sshBridge.cjs.
|
||||
* @param {Function} sendFn - called with the accumulated string to deliver
|
||||
* @returns {{ bufferData: (data: string) => void, flush: () => void }}
|
||||
*/
|
||||
function createPtyBuffer(sendFn) {
|
||||
const FLUSH_INTERVAL = 8; // ms - flush every 8ms (~120fps equivalent)
|
||||
const MAX_BUFFER_SIZE = 16384; // 16KB - flush immediately if buffer grows too large
|
||||
|
||||
let dataBuffer = '';
|
||||
let flushTimeout = null;
|
||||
|
||||
const flushBuffer = () => {
|
||||
if (dataBuffer.length > 0) {
|
||||
sendFn(dataBuffer);
|
||||
dataBuffer = '';
|
||||
}
|
||||
flushTimeout = null;
|
||||
};
|
||||
|
||||
const flush = () => {
|
||||
if (flushTimeout) {
|
||||
clearTimeout(flushTimeout);
|
||||
flushTimeout = null;
|
||||
}
|
||||
flushBuffer();
|
||||
};
|
||||
|
||||
const bufferData = (data) => {
|
||||
dataBuffer += data;
|
||||
if (dataBuffer.length >= MAX_BUFFER_SIZE) {
|
||||
if (flushTimeout) {
|
||||
clearTimeout(flushTimeout);
|
||||
flushTimeout = null;
|
||||
}
|
||||
flushBuffer();
|
||||
} else if (!flushTimeout) {
|
||||
flushTimeout = setTimeout(flushBuffer, FLUSH_INTERVAL);
|
||||
}
|
||||
};
|
||||
|
||||
return { bufferData, flush };
|
||||
}
|
||||
|
||||
/**
|
||||
* Locate an executable on POSIX systems by name.
|
||||
*
|
||||
@@ -454,7 +410,7 @@ function startLocalSession(event, payload) {
|
||||
});
|
||||
}
|
||||
|
||||
const { bufferData: bufferLocalData, flush: flushLocal } = createPtyBuffer((data) => {
|
||||
const { bufferData: bufferLocalData, flush: flushLocal } = createPtyOutputBuffer((data) => {
|
||||
const contents = electronModule.webContents.fromId(session.webContentsId);
|
||||
contents?.send("netcatty:data", { sessionId, data });
|
||||
});
|
||||
@@ -662,7 +618,7 @@ async function startTelnetSession(event, options) {
|
||||
const telnetDecoderRef = { current: iconv.getDecoder(initialTelnetEncoding) };
|
||||
|
||||
const telnetWebContentsId = event.sender.id;
|
||||
const { bufferData: bufferTelnetData, flush: flushTelnet } = createPtyBuffer((data) => {
|
||||
const { bufferData: bufferTelnetData, flush: flushTelnet } = createPtyOutputBuffer((data) => {
|
||||
const contents = electronModule.webContents.fromId(telnetWebContentsId);
|
||||
contents?.send("netcatty:data", { sessionId, data });
|
||||
});
|
||||
@@ -1189,7 +1145,7 @@ async function startMoshSessionViaHandshake(event, options, { bareClient, sshExe
|
||||
// it to scope its stopStream call.
|
||||
session.logStreamToken = logStreamToken;
|
||||
|
||||
const { bufferData, flush } = createPtyBuffer((data) => {
|
||||
const { bufferData, flush } = createPtyOutputBuffer((data) => {
|
||||
const contents = electronModule.webContents.fromId(session.webContentsId);
|
||||
contents?.send("netcatty:data", { sessionId, data });
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user