diff --git a/app/rpc.js b/app/rpc.js index 547f9dcf..e07b9eaf 100644 --- a/app/rpc.js +++ b/app/rpc.js @@ -34,7 +34,11 @@ class Server extends EventEmitter { } emit(ch, data) { - this.wc.send(this.id, {ch, data}); + // This check is needed because data-batching can cause extra data to be + // emitted after the window has already closed + if (!this.win.isDestroyed()) { + this.wc.send(this.id, {ch, data}); + } } destroy() { diff --git a/app/session.js b/app/session.js index 82fd6981..30ff0778 100644 --- a/app/session.js +++ b/app/session.js @@ -21,8 +21,60 @@ try { const envFromConfig = config.getConfig().env || {}; +// Max duration to batch session data before sending it to the renderer process. +const BATCH_DURATION_MS = 16; + +// Max size of a session data batch. Note that this value can be exceeded by ~4k +// (chunk sizes seem to be 4k at the most) +const BATCH_MAX_SIZE = 200 * 1024; + +// Data coming from the pty is sent to the renderer process for further +// vt parsing and rendering. This class batches data to minimize the number of +// IPC calls. It also reduces GC pressure and CPU cost: each chunk is prefixed +// with the window ID which is then stripped on the renderer process and this +// overhead is reduced with batching. +class DataBatcher extends EventEmitter { + constructor(uid) { + super(); + this.uid = uid; + this.decoder = new StringDecoder('utf8'); + + this.reset(); + } + + reset() { + this.data = this.uid; + this.timeout = null; + } + + write(chunk) { + if (this.data.length + chunk.length >= BATCH_MAX_SIZE) { + // We've reached the max batch size. Flush it and start another one + if (this.timeout) { + clearTimeout(this.timeout); + this.timeout = null; + } + this.flush(); + } + + this.data += this.decoder.write(chunk); + + if (!this.timeout) { + this.timeout = setTimeout(() => this.flush(), BATCH_DURATION_MS); + } + } + + flush() { + // Reset before emitting to allow for potential reentrancy + const data = this.data; + this.reset(); + + this.emit('flush', data); + } +} + module.exports = class Session extends EventEmitter { - constructor({rows, cols: columns, cwd, shell, shellArgs}) { + constructor({uid, rows, cols: columns, cwd, shell, shellArgs}) { const osLocale = require('os-locale'); super(); const baseEnv = Object.assign( @@ -45,8 +97,6 @@ module.exports = class Session extends EventEmitter { delete baseEnv.GOOGLE_API_KEY; } - const decoder = new StringDecoder('utf8'); - const defaultShellArgs = ['--login']; try { @@ -64,11 +114,16 @@ module.exports = class Session extends EventEmitter { } } - this.pty.on('data', data => { + this.batcher = new DataBatcher(uid); + this.pty.on('data', chunk => { if (this.ended) { return; } - this.emit('data', decoder.write(data)); + this.batcher.write(chunk); + }); + + this.batcher.on('flush', data => { + this.emit('data', data); }); this.pty.on('exit', () => { diff --git a/app/ui/window.js b/app/ui/window.js index 3edf72b0..e8aca422 100644 --- a/app/ui/window.js +++ b/app/ui/window.js @@ -111,7 +111,7 @@ module.exports = class Window { function createInitialSession() { let {session, options} = createSession(); const initialEvents = []; - const handleData = data => initialEvents.push(['session data', options.uid + data]); + const handleData = data => initialEvents.push(['session data', data]); const handleExit = () => initialEvents.push(['session exit']); session.on('data', handleData); session.on('exit', handleExit); @@ -148,7 +148,7 @@ module.exports = class Window { } session.on('data', data => { - rpc.emit('session data', options.uid + data); + rpc.emit('session data', data); }); session.on('exit', () => {