Skip to content

Commit

Permalink
Add perf optimizations (#1541)
Browse files Browse the repository at this point in the history
* pass `uid` to term

* term: keep track of term instance references

* sessions: remvoe `write` object overhead

* hterm: cache measurement of codepoints for single char strings

* sessions: merge less eagerly when we receive a PTY_DATA action

* store: handle the side effect of writing to the terminal from a middleware

* terms: add terms instance cache

* lint
  • Loading branch information
rauchg authored and matheuss committed Feb 23, 2017
1 parent 1cd2620 commit dc3b900
Show file tree
Hide file tree
Showing 9 changed files with 54 additions and 26 deletions.
3 changes: 2 additions & 1 deletion lib/components/term-group.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ class TermGroup_ extends Component {
onData: this.bind(this.props.onData, null, uid),
onURLAbort: this.bind(this.props.onURLAbort, null, uid),
borderColor: this.props.borderColor,
quickEdit: this.props.quickEdit
quickEdit: this.props.quickEdit,
uid
});

// This will create a new ref_ function for every render,
Expand Down
3 changes: 3 additions & 0 deletions lib/components/term.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import uuid from 'uuid';
import hterm from '../hterm';
import Component from '../component';
import getColorList from '../utils/colors';
import terms from '../terms';
import notify from '../utils/notify';

export default class Term extends Component {
Expand Down Expand Up @@ -97,6 +98,7 @@ export default class Term extends Component {

this.getScreenNode().addEventListener('mouseup', this.handleMouseUp);
this.getScreenNode().addEventListener('mousedown', this.handleMouseDown);
terms[this.props.uid] = this;
}

handleWheel(e) {
Expand Down Expand Up @@ -364,6 +366,7 @@ export default class Term extends Component {
}

componentWillUnmount() {
terms[this.props.uid] = this;
// turn blinking off to prevent leaking a timeout when disposing terminal
const prefs = this.term.getPrefs();
prefs.set('cursor-blink', false);
Expand Down
7 changes: 0 additions & 7 deletions lib/components/terms.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,6 @@ export default class Terms extends Component {
props.ref_(this);
}

componentWillReceiveProps(next) {
const {write} = next;
if (write && this.props.write !== write) {
this.getTermByUid(write.uid).write(write.data);
}
}

shouldComponentUpdate(nextProps) {
for (const i in nextProps) {
if (i === 'write') {
Expand Down
8 changes: 8 additions & 0 deletions lib/hterm.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,12 @@ hterm.TextAttributes.splitWidecharString = function (str) {
};

// hterm Unicode patch
const cache = [];
lib.wc.strWidth = function (str) {
const shouldCache = str.length === 1;
if (shouldCache && cache[str] !== undefined) {
return cache[str];
}
const chars = runes(str);
let width = 0;
let rv = 0;
Expand All @@ -70,6 +75,9 @@ lib.wc.strWidth = function (str) {
}
rv += width * ((codePoint <= 0xffff) ? 1 : 2);
}
if (shouldCache) {
cache[str] = rv;
}
return rv;
};

Expand Down
30 changes: 13 additions & 17 deletions lib/reducers/sessions.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import {

const initialState = Immutable({
sessions: {},
write: null,
activeUid: null
});

Expand All @@ -26,21 +25,13 @@ function Session(obj) {
title: '',
cols: null,
rows: null,
write: null,
url: null,
cleared: false,
shell: '',
pid: null
}).merge(obj);
}

function Write(obj) {
return Immutable({
uid: '',
data: ''
}).merge(obj);
}

const reducer = (state = initialState, action) => {
switch (action.type) {
case SESSION_ADD:
Expand Down Expand Up @@ -73,15 +64,20 @@ const reducer = (state = initialState, action) => {
}, {deep: true});

case SESSION_PTY_DATA:
return state
.set('write', Write(action))
.merge({
sessions: {
[action.uid]: {
cleared: false
// we avoid a direct merge for perf reasons
// as this is the most common action
if (state.sessions[action.uid] &&
state.sessions[action.uid].cleared) {
return state
.merge({
sessions: {
[action.uid]: {
cleared: false
}
}
}
}, {deep: true});
}, {deep: true});
}
return state;

case SESSION_PTY_EXIT:
if (state.sessions[action.uid]) {
Expand Down
2 changes: 2 additions & 0 deletions lib/store/configure-store.dev.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import createLogger from 'redux-logger';
import rootReducer from '../reducers/index';
import effects from '../utils/effects';
import * as plugins from '../utils/plugins';
import writeMiddleware from './write-middleware';

export default () => {
const logger = createLogger({
Expand All @@ -17,6 +18,7 @@ export default () => {
plugins.middleware,
thunk,
effects,
writeMiddleware,
logger
),
window.devToolsExtension()
Expand Down
4 changes: 3 additions & 1 deletion lib/store/configure-store.prod.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import thunk from 'redux-thunk';
import rootReducer from '../reducers/index';
import effects from '../utils/effects';
import * as plugins from '../utils/plugins';
import writeMiddleware from './write-middleware';

export default () =>
createStore(
Expand All @@ -11,6 +12,7 @@ export default () =>
thunk,
plugins.middleware,
thunk,
effects
effects,
writeMiddleware
)
);
14 changes: 14 additions & 0 deletions lib/store/write-middleware.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import terms from '../terms';

// the only side effect we perform from middleware
// is to write to the react term instance directly
// to avoid a performance hit
export default () => next => action => {
if (action.type === 'SESSION_PTY_DATA') {
const term = terms[action.uid];
if (term) {
term.write(action.data);
}
}
next(action);
};
9 changes: 9 additions & 0 deletions lib/terms.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// react Term components add themselves
// to this object upon mounting / unmounting
// this is to allow imperative access to the
// term API, which is a performance
// optimization for the most common action
// within the system

const terms = {};
export default terms;

0 comments on commit dc3b900

Please sign in to comment.