From 53757ea9d406746545dc82c9137d90ef9cc55b01 Mon Sep 17 00:00:00 2001 From: kpdecker Date: Sat, 25 Jan 2014 13:25:08 -0600 Subject: [PATCH 01/13] Implement domEach utility Optimizes internal iteration for both memory and speed by avoiding creation of Cheerio instances when operating directly on DOM elements. --- lib/api/attributes.js | 9 +++++---- lib/api/manipulation.js | 33 +++++++++++++++++++-------------- lib/utils.js | 13 +++++++++++++ 3 files changed, 37 insertions(+), 18 deletions(-) diff --git a/lib/api/attributes.js b/lib/api/attributes.js index edc22f1924..fc51ca2fb5 100644 --- a/lib/api/attributes.js +++ b/lib/api/attributes.js @@ -1,8 +1,9 @@ -var _ = require('underscore'), +var _ = require('lodash'), utils = require('../utils'), isTag = utils.isTag, decode = utils.decode, encode = utils.encode, + domEach = utils.domEach, hasOwn = Object.prototype.hasOwnProperty, rspace = /\s+/, @@ -40,7 +41,7 @@ var attr = exports.attr = function(name, value) { setAttr(el, name, value.call(this, i, el.attribs[name])); }); } - return this.each(function(i, el) { + return domEach(this, function(i, el) { el.attribs = setAttr(el, name, value); }); } @@ -218,7 +219,7 @@ var removeAttribute = function(elem, name) { var removeAttr = exports.removeAttr = function(name) { - this.each(function(i, elem) { + domEach(this, function(i, elem) { removeAttribute(elem, name); }); @@ -292,7 +293,7 @@ var removeClass = exports.removeClass = function(value) { classes = split(value); removeAll = arguments.length === 0; - return this.each(function(i, el) { + return domEach(this, function(i, el) { if (!isTag(el)) return; el.attribs.class = removeAll ? '' : diff --git a/lib/api/manipulation.js b/lib/api/manipulation.js index 35bfc3b571..b142fb3adb 100644 --- a/lib/api/manipulation.js +++ b/lib/api/manipulation.js @@ -3,9 +3,10 @@ var _ = require('underscore'), $ = require('../static'), updateDOM = parse.update, evaluate = parse.evaluate, - encode = require('../utils').encode, + utils = require('../utils'), + domEach = utils.domEach, + encode = utils.encode, slice = Array.prototype.slice; - // Create an array of nodes, recursing into arrays and parsing strings if // necessary var makeDomArray = function(elem) { @@ -27,12 +28,16 @@ var _insert = function(concatenator) { var elems = slice.call(arguments), dom = makeDomArray(elems); - return this.each(function(i, el) { - if (_.isFunction(elems[0])) { + if (_.isFunction(elems[0])) { + return this.each(function(i, el) { dom = makeDomArray(elems[0].call(el, i, this.html())); - } - updateDOM(concatenator(dom, el.children || (el.children = [])), el); - }); + updateDOM(concatenator(dom, el.children || (el.children = [])), el); + }); + } else { + return domEach(this, function(i, el) { + updateDOM(concatenator(dom, el.children || (el.children = [])), el); + }); + } }; }; @@ -48,7 +53,7 @@ var after = exports.after = function() { var elems = slice.call(arguments), dom = makeDomArray(elems); - this.each(function(i, el) { + domEach(this, function(i, el) { var parent = el.parent || el.root, siblings = parent.children, index = siblings.indexOf(el); @@ -74,7 +79,7 @@ var before = exports.before = function() { var elems = slice.call(arguments), dom = makeDomArray(elems); - this.each(function(i, el) { + domEach(this, function(i, el) { var parent = el.parent || el.root, siblings = parent.children, index = siblings.indexOf(el); @@ -106,7 +111,7 @@ var remove = exports.remove = function(selector) { if (selector) elems = elems.filter(selector); - elems.each(function(i, el) { + domEach(elems, function(i, el) { var parent = el.parent || el.root, siblings = parent.children, index = siblings.indexOf(el); @@ -125,7 +130,7 @@ var remove = exports.remove = function(selector) { var replaceWith = exports.replaceWith = function(content) { var dom = makeDomArray(content); - this.each(function(i, el) { + domEach(this, function(i, el) { var parent = el.parent || el.root, siblings = parent.children, index; @@ -151,7 +156,7 @@ var replaceWith = exports.replaceWith = function(content) { }; var empty = exports.empty = function() { - this.each(function(i, el) { + domEach(this, function(i, el) { el.children = []; }); return this; @@ -168,7 +173,7 @@ var html = exports.html = function(str) { str = str.cheerio ? str.toArray() : evaluate(str); - this.each(function(i, el) { + domEach(this, function(i, el) { el.children = str; updateDOM(el.children, el); }); @@ -201,7 +206,7 @@ var text = exports.text = function(str) { }; // Append text node to each selected elements - this.each(function(i, el) { + domEach(this, function(i, el) { el.children = elem; updateDOM(el.children, el); }); diff --git a/lib/utils.js b/lib/utils.js index 54afaaed69..64a55393a1 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -40,3 +40,16 @@ exports.camelCase = function(str) { exports.encode = function(str) { return entities.encode(String(str), 0); }; exports.decode = function(str) { return entities.decode(str, 2); }; + +/** + * Iterate over each DOM element without creating intermediary Cheerio instances. + * + * This is indented for use internally to avoid otherwise unnecessary memory pressure introduced + * by _make. + */ + +exports.domEach = function(cheerio, fn) { + var i = 0, len = cheerio.length; + while (i < len && fn(i, cheerio[i]) !== false) ++i; + return cheerio; +}; From eaa0faa72b8f3dd562156b1cafe4a513e1468a7d Mon Sep 17 00:00:00 2001 From: kpdecker Date: Sat, 25 Jan 2014 13:43:48 -0600 Subject: [PATCH 02/13] Implement getAttr local helper --- lib/api/attributes.js | 42 ++++++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/lib/api/attributes.js b/lib/api/attributes.js index fc51ca2fb5..5c0a7c6a23 100644 --- a/lib/api/attributes.js +++ b/lib/api/attributes.js @@ -21,6 +21,27 @@ var _ = require('lodash'), rbrace = /^(?:\{[\w\W]*\}|\[[\w\W]*\])$/; +var getAttr = function(elem, name) { + if (!elem || !isTag(elem)) return; + + if (!elem.attribs) { + elem.attribs = {}; + } + + // Return the entire attribs object if no attribute specified + if (!name) { + for (var a in elem.attribs) { + elem.attribs[a] = decode(elem.attribs[a]); + } + return elem.attribs; + } + + if (hasOwn.call(elem.attribs, name)) { + // Get the (decoded) attribute + return decode(elem.attribs[name]); + } +}; + var setAttr = function(el, name, value) { if (typeof name === 'object') return _.extend(el.attribs, name); @@ -46,26 +67,7 @@ var attr = exports.attr = function(name, value) { }); } - var elem = this[0]; - - if (!elem || !isTag(elem)) return; - - if (!elem.attribs) { - elem.attribs = {}; - } - - // Return the entire attribs object if no attribute specified - if (!name) { - for (var a in elem.attribs) { - elem.attribs[a] = decode(elem.attribs[a]); - } - return elem.attribs; - } - - if (hasOwn.call(elem.attribs, name)) { - // Get the (decoded) attribute - return decode(elem.attribs[name]); - } + return getAttr(this[0], name); }; var setData = function(el, name, value) { From efb92b9f17b7de823a8ba9c0e7265825abd5567a Mon Sep 17 00:00:00 2001 From: kpdecker Date: Sat, 25 Jan 2014 13:44:08 -0600 Subject: [PATCH 03/13] Avoid cheerio creation in add/remove/toggleClass --- lib/api/attributes.js | 54 ++++++++++++++++++++++++++----------------- 1 file changed, 33 insertions(+), 21 deletions(-) diff --git a/lib/api/attributes.js b/lib/api/attributes.js index 5c0a7c6a23..2a2ba93747 100644 --- a/lib/api/attributes.js +++ b/lib/api/attributes.js @@ -248,22 +248,22 @@ var addClass = exports.addClass = function(value) { if (!value || !_.isString(value)) return this; var classNames = value.split(rspace), - numElements = this.length, - numClasses, - setClass, - $elem; + numElements = this.length; for (var i = 0; i < numElements; i++) { - $elem = this._make(this[i]); // If selected element isnt a tag, move on - if (!isTag(this[i])) continue; + if (!isTag(this[i])) return; // If we don't already have classes - if (!$elem.attr('class')) { - $elem.attr('class', classNames.join(' ').trim()); + var className = getAttr(this[i], 'class'), + numClasses, + setClass; + + if (!className) { + setAttr(this[i], 'class', classNames.join(' ').trim()); } else { - setClass = ' ' + $elem.attr('class') + ' '; + setClass = ' ' + className + ' '; numClasses = classNames.length; // Check if class already exists @@ -272,17 +272,18 @@ var addClass = exports.addClass = function(value) { setClass += classNames[j] + ' '; } - $elem.attr('class', setClass.trim()); + setAttr(this[i], 'class', setClass.trim()); } } return this; }; +var splitClass = function(className) { + return className ? className.trim().split(rspace) : []; +}; + var removeClass = exports.removeClass = function(value) { - var split = function(className) { - return className ? className.trim().split(rspace) : []; - }; var classes, removeAll; // Handle if value is a function @@ -292,14 +293,15 @@ var removeClass = exports.removeClass = function(value) { }); } - classes = split(value); + classes = splitClass(value); removeAll = arguments.length === 0; return domEach(this, function(i, el) { if (!isTag(el)) return; + el.attribs.class = removeAll ? '' : - _.difference(split(el.attribs.class), classes).join(' '); + _.difference(splitClass(el.attribs.class), classes).join(' '); }); }; @@ -318,20 +320,30 @@ var toggleClass = exports.toggleClass = function(value, stateVal) { numClasses = classNames.length, isBool = typeof stateVal === 'boolean', numElements = this.length, - $elem, - state; + elementClasses, + index; for (var i = 0; i < numElements; i++) { - $elem = this._make(this[i]); // If selected element isnt a tag, move on if (!isTag(this[i])) continue; + elementClasses = splitClass(this[i].attribs.class); + // Check if class already exists for (var j = 0; j < numClasses; j++) { - // check each className given, space separated list - state = isBool ? stateVal : !$elem.hasClass(classNames[j]); - $elem[state ? 'addClass' : 'removeClass'](classNames[j]); + // Check if the class name is curently defined + index = elementClasses.indexOf(classNames[j]); + + // Add if stateValue === true or we are toggling and there is no value + if (isBool ? stateVal : index < 0) { + elementClasses.push(classNames[j]); + } else if (index >= 0) { + // Otherwise remove but only if the item exists + elementClasses.splice(index, 1); + } } + + this[i].attribs.class = elementClasses.join(' '); } return this; From 2ba54071a735010613049111d6708c5ab2aa7de0 Mon Sep 17 00:00:00 2001 From: kpdecker Date: Sat, 25 Jan 2014 16:22:46 -0600 Subject: [PATCH 04/13] Use domEach in data accessor --- lib/api/attributes.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/api/attributes.js b/lib/api/attributes.js index 2a2ba93747..d1a36e6acf 100644 --- a/lib/api/attributes.js +++ b/lib/api/attributes.js @@ -105,7 +105,7 @@ var data = exports.data = function(name, value) { // Set the value (with attr map support) if (typeof name === 'object' || value !== undefined) { - this.each(function(i, el) { + domEach(this, function(i, el) { el.data = setData(el, name, value); }); return this; From 4ed048885d850191be9c1dcad9b72f996ff4bb34 Mon Sep 17 00:00:00 2001 From: kpdecker Date: Sat, 25 Jan 2014 16:23:57 -0600 Subject: [PATCH 05/13] Inline children lookup in find --- lib/api/traversing.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/api/traversing.js b/lib/api/traversing.js index 521489c1fe..b937d1830a 100644 --- a/lib/api/traversing.js +++ b/lib/api/traversing.js @@ -4,7 +4,11 @@ var _ = require('underscore'), isTag = utils.isTag; var find = exports.find = function(selector) { - return this._make(select(selector, [].slice.call(this.children()))); + var elems = _.reduce(this, function(memo, elem) { + return memo.concat(_.filter(elem.children, isTag)); + }, []); + + return this._make(select(selector, elems)); }; // Get the parent of each element in the current set of matched elements, From 7ebf0084b3c166cca526ba2a8d93150c577bf7ee Mon Sep 17 00:00:00 2001 From: kpdecker Date: Sat, 25 Jan 2014 16:25:42 -0600 Subject: [PATCH 06/13] Use domEach for traversal --- lib/api/traversing.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/api/traversing.js b/lib/api/traversing.js index b937d1830a..530b7cec15 100644 --- a/lib/api/traversing.js +++ b/lib/api/traversing.js @@ -1,6 +1,7 @@ var _ = require('underscore'), select = require('CSSselect'), utils = require('../utils'), + domEach = utils.domEach, isTag = utils.isTag; var find = exports.find = function(selector) { @@ -17,7 +18,7 @@ var parent = exports.parent = function(selector) { var set = []; var $set; - this.each(function(idx, elem) { + domEach(this, function(idx, elem) { var parentElem = elem.parent; if (parentElem && set.indexOf(parentElem) < 0) { set.push(parentElem); @@ -62,7 +63,7 @@ var closest = exports.closest = function(selector) { return this._make(set); } - this.each(function(idx, elem) { + domEach(this, function(idx, elem) { var closestElem = traverseParents(this, elem, selector, 1)[0]; // Do not add duplicate elements to the set From f4e5d436d8ecb8e5784b40aa61ad602426b0b7f5 Mon Sep 17 00:00:00 2001 From: kpdecker Date: Sat, 25 Jan 2014 16:26:25 -0600 Subject: [PATCH 07/13] Optimize identity cases for first/last/eq --- lib/api/traversing.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/api/traversing.js b/lib/api/traversing.js index 530b7cec15..d1a3b4a4be 100644 --- a/lib/api/traversing.js +++ b/lib/api/traversing.js @@ -263,16 +263,20 @@ var filter = exports.filter = function(match) { }; var first = exports.first = function() { - return this[0] ? this._make(this[0]) : this; + return this.length > 1 ? this._make(this[0]) : this; }; var last = exports.last = function() { - return this[0] ? this._make(this[this.length - 1]) : this; + return this.length > 1 ? this._make(this[this.length - 1]) : this; }; // Reduce the set of matched elements to the one at the specified index. var eq = exports.eq = function(i) { i = +i; + + // Use the first identity optimization if possible + if (!i && this.length <= 1) return this; + if (i < 0) i = this.length + i; return this[i] ? this._make(this[i]) : this._make([]); }; From 3316457818ec00edd93349d6b283c601118ace45 Mon Sep 17 00:00:00 2001 From: kpdecker Date: Sat, 25 Jan 2014 16:27:40 -0600 Subject: [PATCH 08/13] Optimize siblings cheerio instance creation --- lib/api/traversing.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/api/traversing.js b/lib/api/traversing.js index d1a3b4a4be..cc3a2143d0 100644 --- a/lib/api/traversing.js +++ b/lib/api/traversing.js @@ -196,15 +196,19 @@ var prevUntil = exports.prevUntil = function(selector, filter) { }; var siblings = exports.siblings = function(selector) { + var parent = this.parent(); + var elems = _.filter( - this.parent() ? this.parent().children() : this.siblingsAndMe(), + parent ? parent.children() : this.siblingsAndMe(), function(elem) { return isTag(elem) && !this.is(elem); }, this ); + if (selector !== undefined) { - elems = this._make(select(selector, elems)); + return this._make(select(selector, elems)); + } else { + return this._make(elems); } - return this._make(elems); }; var children = exports.children = function(selector) { From f3cbcfeef66ba2759bc947f5c07662666e8cd43e Mon Sep 17 00:00:00 2001 From: kpdecker Date: Sat, 25 Jan 2014 16:28:57 -0600 Subject: [PATCH 09/13] Optimize filter Cheerio intermediate creation --- lib/api/traversing.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/api/traversing.js b/lib/api/traversing.js index cc3a2143d0..c565cfd03c 100644 --- a/lib/api/traversing.js +++ b/lib/api/traversing.js @@ -25,13 +25,11 @@ var parent = exports.parent = function(selector) { } }); - $set = this._make(set); - if (arguments.length) { - $set = $set.filter(selector); + set = filter.call(set, selector, this); } - return $set; + return this._make(set); }; var parents = exports.parents = function(selector) { @@ -220,7 +218,7 @@ var children = exports.children = function(selector) { if (selector === undefined) return this._make(elems); else if (_.isNumber(selector)) return this._make(elems[selector]); - return this._make(elems).filter(selector); + return filter.call(elems, selector, this); }; var contents = exports.contents = function() { @@ -243,8 +241,10 @@ var map = exports.map = function(fn) { }, [])); }; -var filter = exports.filter = function(match) { - var make = _.bind(this._make, this); +var filter = exports.filter = function(match, container) { + container = container || this; + + var make = _.bind(container._make, container); var filterFn; if (_.isString(match)) { @@ -292,7 +292,7 @@ var slice = exports.slice = function() { function traverseParents(self, elem, selector, limit) { var elems = []; while (elem && elems.length < limit) { - if (!selector || self._make(elem).filter(selector).length) { + if (!selector || filter.call([elem], selector, self).length) { elems.push(elem); } elem = elem.parent; From c81601e0b26a0d707b386a1fd2e78137053d5538 Mon Sep 17 00:00:00 2001 From: kpdecker Date: Sat, 25 Jan 2014 16:36:30 -0600 Subject: [PATCH 10/13] Remove erroneous lodash reference --- lib/api/attributes.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/api/attributes.js b/lib/api/attributes.js index d1a36e6acf..16dc9fad1c 100644 --- a/lib/api/attributes.js +++ b/lib/api/attributes.js @@ -1,4 +1,4 @@ -var _ = require('lodash'), +var _ = require('underscore'), utils = require('../utils'), isTag = utils.isTag, decode = utils.decode, From ccff7167e2717f26cb0977ce5939cebcc4308d5b Mon Sep 17 00:00:00 2001 From: kpdecker Date: Sat, 25 Jan 2014 16:39:33 -0600 Subject: [PATCH 11/13] s/return/continue/ --- lib/api/attributes.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/api/attributes.js b/lib/api/attributes.js index 16dc9fad1c..57b47b6c41 100644 --- a/lib/api/attributes.js +++ b/lib/api/attributes.js @@ -253,7 +253,7 @@ var addClass = exports.addClass = function(value) { for (var i = 0; i < numElements; i++) { // If selected element isnt a tag, move on - if (!isTag(this[i])) return; + if (!isTag(this[i])) continue; // If we don't already have classes var className = getAttr(this[i], 'class'), From dfd059503301ecea3ca70cb86ec2851ce2a0f3e3 Mon Sep 17 00:00:00 2001 From: kpdecker Date: Wed, 29 Jan 2014 11:15:14 -0800 Subject: [PATCH 12/13] Use strict equality rather than falsy check in eq --- lib/api/traversing.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/api/traversing.js b/lib/api/traversing.js index c565cfd03c..075092dbf1 100644 --- a/lib/api/traversing.js +++ b/lib/api/traversing.js @@ -279,7 +279,7 @@ var eq = exports.eq = function(i) { i = +i; // Use the first identity optimization if possible - if (!i && this.length <= 1) return this; + if (i === 0 && this.length <= 1) return this; if (i < 0) i = this.length + i; return this[i] ? this._make(this[i]) : this._make([]); From 9e0ebe701029ee984f3fca838c0ee3ad9e6712b3 Mon Sep 17 00:00:00 2001 From: kpdecker Date: Wed, 29 Jan 2014 11:26:17 -0800 Subject: [PATCH 13/13] Avoid unnecessary indexOf from toggleClass --- lib/api/attributes.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/api/attributes.js b/lib/api/attributes.js index 57b47b6c41..f6d1093d4a 100644 --- a/lib/api/attributes.js +++ b/lib/api/attributes.js @@ -332,7 +332,7 @@ var toggleClass = exports.toggleClass = function(value, stateVal) { // Check if class already exists for (var j = 0; j < numClasses; j++) { // Check if the class name is curently defined - index = elementClasses.indexOf(classNames[j]); + index = !isBool || !stateVal ? elementClasses.indexOf(classNames[j]) : -1; // Add if stateValue === true or we are toggling and there is no value if (isBool ? stateVal : index < 0) {