Skip to content

Commit c07700a

Browse files
juancamparauchg
authored andcommitted
Performance - Data batching (#3336)
* Bumping electron to 3.0.10 * Updating node version in travis and appveyor * Fixing incorrect require of electron-fetch * Fix zoom to match previous versions Additionally I'm removing a call to disable pinch-zoom, it's disable by default since Electron 2 (https://electronjs.org/releases#2.0.0) * Bumping electron to 4.0.0-beta.8 * Bumping electron to 4.0.0-beta.9 * Work around for Copy accelerator not firing on electron v4 * Batch session data before sending it to renderer * Fix linting issues * Fixing header/titlebar in MacOS * Upgrading to electron 4.0.0 and node-pty 0.8.0 * Adding yarn.lock changes for electron 4.0.0 * Adding comments for editor:copy workaround. Scaling issue is only on Linux * Upgrading node-abi to support electron 4.0.0 * Adding isDestroyed check
1 parent 7a40fd7 commit c07700a

File tree

3 files changed

+67
-8
lines changed

3 files changed

+67
-8
lines changed

app/rpc.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,11 @@ class Server extends EventEmitter {
3434
}
3535

3636
emit(ch, data) {
37-
this.wc.send(this.id, {ch, data});
37+
// This check is needed because data-batching can cause extra data to be
38+
// emitted after the window has already closed
39+
if (!this.win.isDestroyed()) {
40+
this.wc.send(this.id, {ch, data});
41+
}
3842
}
3943

4044
destroy() {

app/session.js

Lines changed: 60 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,60 @@ try {
2121

2222
const envFromConfig = config.getConfig().env || {};
2323

24+
// Max duration to batch session data before sending it to the renderer process.
25+
const BATCH_DURATION_MS = 16;
26+
27+
// Max size of a session data batch. Note that this value can be exceeded by ~4k
28+
// (chunk sizes seem to be 4k at the most)
29+
const BATCH_MAX_SIZE = 200 * 1024;
30+
31+
// Data coming from the pty is sent to the renderer process for further
32+
// vt parsing and rendering. This class batches data to minimize the number of
33+
// IPC calls. It also reduces GC pressure and CPU cost: each chunk is prefixed
34+
// with the window ID which is then stripped on the renderer process and this
35+
// overhead is reduced with batching.
36+
class DataBatcher extends EventEmitter {
37+
constructor(uid) {
38+
super();
39+
this.uid = uid;
40+
this.decoder = new StringDecoder('utf8');
41+
42+
this.reset();
43+
}
44+
45+
reset() {
46+
this.data = this.uid;
47+
this.timeout = null;
48+
}
49+
50+
write(chunk) {
51+
if (this.data.length + chunk.length >= BATCH_MAX_SIZE) {
52+
// We've reached the max batch size. Flush it and start another one
53+
if (this.timeout) {
54+
clearTimeout(this.timeout);
55+
this.timeout = null;
56+
}
57+
this.flush();
58+
}
59+
60+
this.data += this.decoder.write(chunk);
61+
62+
if (!this.timeout) {
63+
this.timeout = setTimeout(() => this.flush(), BATCH_DURATION_MS);
64+
}
65+
}
66+
67+
flush() {
68+
// Reset before emitting to allow for potential reentrancy
69+
const data = this.data;
70+
this.reset();
71+
72+
this.emit('flush', data);
73+
}
74+
}
75+
2476
module.exports = class Session extends EventEmitter {
25-
constructor({rows, cols: columns, cwd, shell, shellArgs}) {
77+
constructor({uid, rows, cols: columns, cwd, shell, shellArgs}) {
2678
const osLocale = require('os-locale');
2779
super();
2880
const baseEnv = Object.assign(
@@ -45,8 +97,6 @@ module.exports = class Session extends EventEmitter {
4597
delete baseEnv.GOOGLE_API_KEY;
4698
}
4799

48-
const decoder = new StringDecoder('utf8');
49-
50100
const defaultShellArgs = ['--login'];
51101

52102
try {
@@ -64,11 +114,16 @@ module.exports = class Session extends EventEmitter {
64114
}
65115
}
66116

67-
this.pty.on('data', data => {
117+
this.batcher = new DataBatcher(uid);
118+
this.pty.on('data', chunk => {
68119
if (this.ended) {
69120
return;
70121
}
71-
this.emit('data', decoder.write(data));
122+
this.batcher.write(chunk);
123+
});
124+
125+
this.batcher.on('flush', data => {
126+
this.emit('data', data);
72127
});
73128

74129
this.pty.on('exit', () => {

app/ui/window.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ module.exports = class Window {
111111
function createInitialSession() {
112112
let {session, options} = createSession();
113113
const initialEvents = [];
114-
const handleData = data => initialEvents.push(['session data', options.uid + data]);
114+
const handleData = data => initialEvents.push(['session data', data]);
115115
const handleExit = () => initialEvents.push(['session exit']);
116116
session.on('data', handleData);
117117
session.on('exit', handleExit);
@@ -148,7 +148,7 @@ module.exports = class Window {
148148
}
149149

150150
session.on('data', data => {
151-
rpc.emit('session data', options.uid + data);
151+
rpc.emit('session data', data);
152152
});
153153

154154
session.on('exit', () => {

0 commit comments

Comments
 (0)