From 824e6d69bf4c337976e3cd3798857d2c6132fd30 Mon Sep 17 00:00:00 2001 From: Will Thompson Date: Thu, 30 Apr 2015 21:36:48 +0100 Subject: [PATCH 1/2] highlighter: remove redundant (?) call to $.show() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I don't understand why this would be necessary -- the element's just been created, no style is (presumably) changing its display to anything other than the default... -- and when highlighting all text in a 5000×3 table, $.show() accounts for 10s out of the 14s it takes to draw the highlights. $.show() being absurdly slow is a known problem: https://github.com/jquery/jquery.com/issues/88 --- src/ui/highlighter.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ui/highlighter.js b/src/ui/highlighter.js index 0d10cca6e..e8fcfe837 100644 --- a/src/ui/highlighter.js +++ b/src/ui/highlighter.js @@ -34,7 +34,7 @@ function highlightRange(normedRange, cssClass) { var node = nodes[i]; if (!white.test(node.nodeValue)) { results.push( - $(node).wrapAll(hl).parent().show()[0] + $(node).wrapAll(hl).parent()[0] ); } } From 2b2700eb251d6f96d18452e1d84dc8105fa949bc Mon Sep 17 00:00:00 2001 From: Will Thompson Date: Thu, 30 Apr 2015 22:07:11 +0100 Subject: [PATCH 2/2] Add span.annotator-hl wrappers without jQuery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This funtion accounted for ~1 second of the remaining time in my 5000×3 table test case. With this change it's more like 200ms, which is still not ideal but a lot better. $.wrapAll seems relatively expensive. I suppose it has to do lots of extra work that's not necessary when we know we have exactly one element to wrap at a time. --- src/ui/highlighter.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/ui/highlighter.js b/src/ui/highlighter.js index e8fcfe837..b1e93dc5b 100644 --- a/src/ui/highlighter.js +++ b/src/ui/highlighter.js @@ -21,8 +21,6 @@ function highlightRange(normedRange, cssClass) { } var white = /^\s*$/; - var hl = $(""); - // Ignore text nodes that contain only whitespace characters. This prevents // spans being injected between elements that can only contain a restricted // subset of nodes such as table rows and lists. This does mean that there @@ -33,9 +31,11 @@ function highlightRange(normedRange, cssClass) { for (var i = 0, len = nodes.length; i < len; i++) { var node = nodes[i]; if (!white.test(node.nodeValue)) { - results.push( - $(node).wrapAll(hl).parent()[0] - ); + var hl = document.createElement('span'); + hl.className = cssClass; + node.parentNode.replaceChild(hl, node); + hl.appendChild(node); + results.push(hl); } } return results;