Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Improve TokenUtils performance through caching #9964

Merged
merged 1 commit into from
Dec 19, 2014
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 60 additions & 7 deletions src/utils/TokenUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,20 +33,73 @@
define(function (require, exports, module) {
"use strict";

var CodeMirror = require("thirdparty/CodeMirror2/lib/codemirror");
var _ = require("thirdparty/lodash"),
CodeMirror = require("thirdparty/CodeMirror2/lib/codemirror");

var cache;


function _clearCache(cm) {
cache = null;
if (cm) { // event handler
cm.off("changes", _clearCache);
}
}

/*
* Caches the tokens for the given editor/line if needed
* @param {!CodeMirror} cm
* @param {!number} line
* @return {Array.<Object>} (Cached) array of tokens
*/
function _manageCache(cm, line) {
if (!cache || !cache.tokens || cache.line !== line || cache.cm !== cm) {
// Cache is no longer matching -> Update
var tokens = cm.getLineTokens(line, false);
// Add empty beginning-of-line token for backwards compatibility
tokens.unshift(cm.getTokenAt({line: line, ch: 0}, false));
cache = {
cm: cm,
line: line,
timeStamp: Date.now(),
tokens: tokens,
};
cm.off("changes", _clearCache);
cm.on("changes", _clearCache);
}
return cache.tokens;
}

/*
* Like cm.getTokenAt, but with caching
* @param {!CodeMirror} cm
* @param {!{ch:number, line:number}} pos
* @param {boolean} precise If given, results in more current results. Suppresses caching.
* @return {Object} Token for position
*/
function _getToken(cm, pos, precise) {
if (precise) {
_clearCache(); // reset cache
return cm.getTokenAt(pos, precise);
}
var cachedTokens = _manageCache(cm, pos.line),
tokenIndex = _.sortedIndex(cachedTokens, {end: pos.ch}, "end"), // binary search is faster for long arrays
token = cachedTokens[tokenIndex];
return token || cm.getTokenAt(pos, precise); // fall back to CMs getTokenAt, for example in an empty line
}

/**
* Creates a context object for the given editor and position, suitable for passing to the
* move functions.
* @param {!CodeMirror} editor
* @param {!CodeMirror} cm
* @param {!{ch:number, line:number}} pos
* @return {!{editor:!CodeMirror, pos:!{ch:number, line:number}, token:Object}}
*/
function getInitialContext(editor, pos) {
function getInitialContext(cm, pos) {
return {
"editor": editor,
"editor": cm,
"pos": pos,
"token": editor.getTokenAt(pos, true)
"token": cm.getTokenAt(pos, true)
};
}

Expand All @@ -72,7 +125,7 @@ define(function (require, exports, module) {
} else {
ctx.pos.ch = ctx.token.start;
}
ctx.token = ctx.editor.getTokenAt(ctx.pos, precise);
ctx.token = _getToken(ctx.editor, ctx.pos, precise);
return true;
}

Expand Down Expand Up @@ -107,7 +160,7 @@ define(function (require, exports, module) {
} else {
ctx.pos.ch = ctx.token.end + 1;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function was written this way for nested mode support, so we need to make sure that cm.getModeAt(pos) supports that. I'm seeing the HTMLUtils InlineEditorProviders "should find style blocks" unit test fail.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't tested it, but the CM manual said it returns the mode at the given pos.
I'll look into the unit tests and the Travis failure soon.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me, it works.
I checked it in a file with mixed mode (HTML/JS) and the unit tests pass for me as well.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I only brought up TokenUtils.getModeAt() because I thought that it may benefit from the cache, but that does not appear to be the case. To keep these improvements isolated so they can be measured separately, this change should be moved to another pull request.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverted the change.

}
ctx.token = ctx.editor.getTokenAt(ctx.pos, precise);
ctx.token = _getToken(ctx.editor, ctx.pos, precise);
return true;
}

Expand Down