From b836cccb997572735ffd924b9726c29c5857b8f6 Mon Sep 17 00:00:00 2001 From: Leo Horie Date: Fri, 14 Nov 2014 23:35:06 -0500 Subject: [PATCH 1/5] performance optimization: don't add key as an attribute --- mithril.js | 7 +++--- tests/mithril-tests.js | 56 +++++++++++++++++++++--------------------- 2 files changed, 31 insertions(+), 32 deletions(-) diff --git a/mithril.js b/mithril.js index 8a2d1166..155e6909 100644 --- a/mithril.js +++ b/mithril.js @@ -235,7 +235,7 @@ Mithril = m = new function app(window, undefined) { cached = { tag: data.tag, //set attributes first, then create children - attrs: dataAttrKeys.length ? setAttributes(node, data.tag, data.attrs, {}, namespace) : {}, + attrs: hasKeys ? setAttributes(node, data.tag, data.attrs, {}, namespace) : {}, children: data.children != null && data.children.length > 0 ? build(node, data.tag, undefined, undefined, data.children, cached.children, true, 0, data.attrs.contenteditable ? node : editable, namespace, configs) : data.children, @@ -248,7 +248,7 @@ Mithril = m = new function app(window, undefined) { } else { node = cached.nodes[0]; - if (dataAttrKeys.length) setAttributes(node, data.tag, data.attrs, cached.attrs, namespace); + if (hasKeys) setAttributes(node, data.tag, data.attrs, cached.attrs, namespace); cached.children = build(node, data.tag, undefined, undefined, data.children, cached.children, false, 0, data.attrs.contenteditable ? node : editable, namespace, configs); cached.nodes.intact = true; if (shouldReattach === true && node != null) parentElement.insertBefore(node, parentElement.childNodes[index] || null) @@ -318,8 +318,7 @@ Mithril = m = new function app(window, undefined) { cachedAttrs[attrName] = dataAttr; try { //`config` isn't a real attributes, so ignore it - //we don't ignore `key` because it must be unique and having it on the DOM helps debugging - if (attrName === "config") continue; + if (attrName === "config" || attrName == "key") continue; //hook event handlers to the auto-redrawing system else if (typeof dataAttr == sFn && attrName.indexOf("on") == 0) { node[attrName] = autoredraw(dataAttr, node) diff --git a/tests/mithril-tests.js b/tests/mithril-tests.js index 6b3212d5..de9f97be 100644 --- a/tests/mithril-tests.js +++ b/tests/mithril-tests.js @@ -509,55 +509,55 @@ function testMithril(mock) { //https://github.com/lhorie/mithril.js/issues/98 //insert at beginning var root = mock.document.createElement("div") - m.render(root, [m("a", {key: 1}), m("a", {key: 2}), m("a", {key: 3})]) + m.render(root, [m("a", {key: 1}, 1), m("a", {key: 2}, 2), m("a", {key: 3}, 3)]) var firstBefore = root.childNodes[0] - m.render(root, [m("a", {key: 4}), m("a", {key: 1}), m("a", {key: 2}), m("a", {key: 3})]) + m.render(root, [m("a", {key: 4}, 4), m("a", {key: 1}, 1), m("a", {key: 2}, 2), m("a", {key: 3}, 3)]) var firstAfter = root.childNodes[1] - return firstBefore == firstAfter && root.childNodes[0].key == 4 && root.childNodes.length == 4 + return firstBefore == firstAfter && root.childNodes[0].childNodes[0].nodeValue == "4" && root.childNodes.length == 4 }) test(function() { //https://github.com/lhorie/mithril.js/issues/98 var root = mock.document.createElement("div") - m.render(root, [m("a", {key: 1}), m("a", {key: 2}), m("a", {key: 3})]) + m.render(root, [m("a", {key: 1}, 1), m("a", {key: 2}, 2), m("a", {key: 3}, 3)]) var firstBefore = root.childNodes[0] - m.render(root, [m("a", {key: 4}), m("a", {key: 1}), m("a", {key: 2})]) + m.render(root, [m("a", {key: 4}, 4), m("a", {key: 1}, 1), m("a", {key: 2}, 2)]) var firstAfter = root.childNodes[1] - return firstBefore == firstAfter && root.childNodes[0].key == 4 && root.childNodes.length == 3 + return firstBefore == firstAfter && root.childNodes[0].childNodes[0].nodeValue == 4 && root.childNodes.length == 3 }) test(function() { //https://github.com/lhorie/mithril.js/issues/98 var root = mock.document.createElement("div") - m.render(root, [m("a", {key: 1}), m("a", {key: 2}), m("a", {key: 3})]) + m.render(root, [m("a", {key: 1}, 1), m("a", {key: 2}, 2), m("a", {key: 3}, 3)]) var firstBefore = root.childNodes[1] - m.render(root, [m("a", {key: 2}), m("a", {key: 3}), m("a", {key: 4})]) + m.render(root, [m("a", {key: 2}, 2), m("a", {key: 3}, 3), m("a", {key: 4}, 4)]) var firstAfter = root.childNodes[0] - return firstBefore == firstAfter && root.childNodes[0].key === "2" && root.childNodes.length === 3 + return firstBefore == firstAfter && root.childNodes[0].childNodes[0].nodeValue === "2" && root.childNodes.length === 3 }) test(function() { //https://github.com/lhorie/mithril.js/issues/98 var root = mock.document.createElement("div") - m.render(root, [m("a", {key: 1}), m("a", {key: 2}), m("a", {key: 3}), m("a", {key: 4}), m("a", {key: 5})]) + m.render(root, [m("a", {key: 1}, 1), m("a", {key: 2}, 2), m("a", {key: 3}, 3), m("a", {key: 4}, 4), m("a", {key: 5}, 5)]) var firstBefore = root.childNodes[0] var secondBefore = root.childNodes[1] var fourthBefore = root.childNodes[3] - m.render(root, [m("a", {key: 4}), m("a", {key: 10}), m("a", {key: 1}), m("a", {key: 2})]) + m.render(root, [m("a", {key: 4}, 4), m("a", {key: 10}, 10), m("a", {key: 1}, 1), m("a", {key: 2}, 2)]) var firstAfter = root.childNodes[2] var secondAfter = root.childNodes[3] var fourthAfter = root.childNodes[0] - return firstBefore === firstAfter && secondBefore === secondAfter && fourthBefore === fourthAfter && root.childNodes[1].key == "10" && root.childNodes.length === 4 + return firstBefore === firstAfter && secondBefore === secondAfter && fourthBefore === fourthAfter && root.childNodes[1].childNodes[0].nodeValue == "10" && root.childNodes.length === 4 }) test(function() { //https://github.com/lhorie/mithril.js/issues/98 var root = mock.document.createElement("div") - m.render(root, [m("a", {key: 1}), m("a", {key: 2}), m("a", {key: 3}), m("a", {key: 4}), m("a", {key: 5})]) + m.render(root, [m("a", {key: 1}, 1), m("a", {key: 2}, 2), m("a", {key: 3}, 3), m("a", {key: 4}, 4), m("a", {key: 5}, 5)]) var firstBefore = root.childNodes[0] var secondBefore = root.childNodes[1] var fourthBefore = root.childNodes[3] - m.render(root, [m("a", {key: 4}), m("a", {key: 10}), m("a", {key: 2}), m("a", {key: 1}), m("a", {key: 6}), m("a", {key: 7})]) + m.render(root, [m("a", {key: 4}, 4), m("a", {key: 10}, 10), m("a", {key: 2}, 2), m("a", {key: 1}, 1), m("a", {key: 6}, 6), m("a", {key: 7}, 7)]) var firstAfter = root.childNodes[3] var secondAfter = root.childNodes[2] var fourthAfter = root.childNodes[0] - return firstBefore === firstAfter && secondBefore === secondAfter && fourthBefore === fourthAfter && root.childNodes[1].key == "10" && root.childNodes[4].key == "6" && root.childNodes[5].key == "7" && root.childNodes.length === 6 + return firstBefore === firstAfter && secondBefore === secondAfter && fourthBefore === fourthAfter && root.childNodes[1].childNodes[0].nodeValue == "10" && root.childNodes[4].childNodes[0].nodeValue == "6" && root.childNodes[5].childNodes[0].nodeValue == "7" && root.childNodes.length === 6 }) test(function() { //https://github.com/lhorie/mithril.js/issues/149 @@ -580,11 +580,11 @@ function testMithril(mock) { //https://github.com/lhorie/mithril.js/issues/246 //insert at beginning with non-keyed in the middle var root = mock.document.createElement("div") - m.render(root, [m("a", {key: 1})]) + m.render(root, [m("a", {key: 1}, 1)]) var firstBefore = root.childNodes[0] - m.render(root, [m("a", {key: 2}), m("br"), m("a", {key: 1})]) + m.render(root, [m("a", {key: 2}, 2), m("br"), m("a", {key: 1}, 1)]) var firstAfter = root.childNodes[2] - return firstBefore == firstAfter && root.childNodes[0].key == 2 && root.childNodes.length == 3 + return firstBefore == firstAfter && root.childNodes[0].childNodes[0].nodeValue == 2 && root.childNodes.length == 3 }) test(function() { //https://github.com/lhorie/mithril.js/issues/134 @@ -687,9 +687,9 @@ function testMithril(mock) { test(function() { //https://github.com/lhorie/mithril.js/issues/157 var root = mock.document.createElement("div") - m.render(root, m("ul", [m("li", {key: 0}), m("li", {key: 2}), m("li", {key: 4})])) - m.render(root, m("ul", [m("li", {key: 0}), m("li", {key: 1}), m("li", {key: 2}), m("li", {key: 3}), m("li", {key: 4}), m("li", {key: 5})])) - return root.childNodes[0].childNodes.map(function(n) {return n.key}).join("") == "012345" + m.render(root, m("ul", [m("li", {key: 0}, 0), m("li", {key: 2}, 2), m("li", {key: 4}, 4)])) + m.render(root, m("ul", [m("li", {key: 0}, 0), m("li", {key: 1}, 1), m("li", {key: 2}, 2), m("li", {key: 3}, 3), m("li", {key: 4}, 4), m("li", {key: 5}, 5)])) + return root.childNodes[0].childNodes.map(function(n) {return n.childNodes[0].nodeValue}).join("") == "012345" }) test(function() { //https://github.com/lhorie/mithril.js/issues/157 @@ -708,17 +708,17 @@ function testMithril(mock) { test(function() { //https://github.com/lhorie/mithril.js/issues/194 var root = mock.document.createElement("div") - m.render(root, m("ul", [m("li", {key: 0}), m("li", {key: 1}), m("li", {key: 2}), m("li", {key: 3}), m("li", {key: 4}), m("li", {key: 5})])) - m.render(root, m("ul", [m("li", {key: 0}), m("li", {key: 1}), m("li", {key: 2}), m("li", {key: 4}), m("li", {key: 5})])) - return root.childNodes[0].childNodes.map(function(n) {return n.key}).join("") == "01245" + m.render(root, m("ul", [m("li", {key: 0}, 0), m("li", {key: 1}, 1), m("li", {key: 2}, 2), m("li", {key: 3}, 3), m("li", {key: 4}, 4), m("li", {key: 5}, 5)])) + m.render(root, m("ul", [m("li", {key: 0}, 0), m("li", {key: 1}, 1), m("li", {key: 2}, 2), m("li", {key: 4}, 4), m("li", {key: 5}, 5)])) + return root.childNodes[0].childNodes.map(function(n) {return n.childNodes[0].nodeValue}).join("") == "01245" }) test(function() { //https://github.com/lhorie/mithril.js/issues/194 var root = mock.document.createElement("div") - m.render(root, m("ul", [m("li", {key: 0}), m("li", {key: 1}), m("li", {key: 2}), m("li", {key: 3}), m("li", {key: 4}), m("li", {key: 5})])) - m.render(root, m("ul", [m("li", {key: 1}), m("li", {key: 2}), m("li", {key: 3}), m("li", {key: 4}), m("li", {key: 5}), m("li", {key: 6})])) - m.render(root, m("ul", [m("li", {key: 12}), m("li", {key: 13}), m("li", {key: 14}), m("li", {key: 15}), m("li", {key: 16}), m("li", {key: 17})])) - return root.childNodes[0].childNodes.map(function(n) {return n.key}).join(",") == "12,13,14,15,16,17" + m.render(root, m("ul", [m("li", {key: 0}, 0), m("li", {key: 1}, 1), m("li", {key: 2}, 2), m("li", {key: 3}, 3), m("li", {key: 4}, 4), m("li", {key: 5}, 5)])) + m.render(root, m("ul", [m("li", {key: 1}, 1), m("li", {key: 2}, 2), m("li", {key: 3}, 3), m("li", {key: 4}, 4), m("li", {key: 5}, 5), m("li", {key: 6}, 6)])) + m.render(root, m("ul", [m("li", {key: 12}, 12), m("li", {key: 13}, 13), m("li", {key: 14}, 14), m("li", {key: 15}, 15), m("li", {key: 16}, 16), m("li", {key: 17}, 17)])) + return root.childNodes[0].childNodes.map(function(n) {return n.childNodes[0].nodeValue}).join(",") == "12,13,14,15,16,17" }) test(function() { //https://github.com/lhorie/mithril.js/issues/206 From 985611baf4d5cad45fe087b37fc74a99f4692f85 Mon Sep 17 00:00:00 2001 From: Leo Horie Date: Tue, 25 Nov 2014 20:50:11 -0500 Subject: [PATCH 2/5] fix key regression from perf optimization --- mithril.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mithril.js b/mithril.js index 2da6e11f..07c62c4a 100644 --- a/mithril.js +++ b/mithril.js @@ -216,7 +216,7 @@ var m = (function app(window, undefined) { if (!data.attrs) data.attrs = {}; if (!cached.attrs) cached.attrs = {}; - var dataAttrKeys = Object.keys(data.attrs); + var dataAttrKeys = Object.keys(data.attrs) var hasKeys = dataAttrKeys.length > ("key" in data.attrs ? 1 : 0) //if an element is different enough from the one in cache, recreate it if (data.tag != cached.tag || dataAttrKeys.join() != Object.keys(cached.attrs).join() || data.attrs.id != cached.attrs.id) { @@ -235,7 +235,7 @@ var m = (function app(window, undefined) { cached = { tag: data.tag, //set attributes first, then create children - attrs: hasKeys ? setAttributes(node, data.tag, data.attrs, {}, namespace) : {}, + attrs: hasKeys ? setAttributes(node, data.tag, data.attrs, {}, namespace) : data.attrs, children: data.children != null && data.children.length > 0 ? build(node, data.tag, undefined, undefined, data.children, cached.children, true, 0, data.attrs.contenteditable ? node : editable, namespace, configs) : data.children, From b6693c7f0633e680cb0ea1601eb35901d3c1aae1 Mon Sep 17 00:00:00 2001 From: Leo Horie Date: Tue, 25 Nov 2014 21:27:55 -0500 Subject: [PATCH 3/5] decrease memory reallocations --- mithril.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/mithril.js b/mithril.js index 07c62c4a..01244e0f 100644 --- a/mithril.js +++ b/mithril.js @@ -148,9 +148,10 @@ var m = (function app(window, undefined) { else unkeyed.push({index: i, element: parentElement.childNodes[i] || $document.createElement("div")}) } } - var actions = Object.keys(existing).map(function(key) {return existing[key]}); - var changes = actions.sort(function(a, b) {return a.action - b.action || a.index - b.index}); - var newCached = cached.slice(); + var actions = [] + for (var prop in existing) actions.push(existing[prop]) + var changes = actions.sort(sortChanges); + var newCached = new Array(cached.length) for (var i = 0, change; change = changes[i]; i++) { if (change.action == DELETION) { @@ -177,8 +178,8 @@ var m = (function app(window, undefined) { newCached[change.index] = cached[change.index] } cached = newCached; - cached.nodes = []; - for (var i = 0, child; child = parentElement.childNodes[i]; i++) cached.nodes.push(child) + cached.nodes = new Array(parentElement.childNodes.length); + for (var i = 0, child; child = parentElement.childNodes[i]; i++) cached.nodes[i] = child } //end key algorithm @@ -310,6 +311,7 @@ var m = (function app(window, undefined) { return cached } + function sortChanges(a, b) {return a.action - b.action || a.index - b.index} function setAttributes(node, tag, dataAttrs, cachedAttrs, namespace) { for (var attrName in dataAttrs) { var dataAttr = dataAttrs[attrName]; From c9c0250f127cfb00f72321c891cf06d99966f70a Mon Sep 17 00:00:00 2001 From: Leo Horie Date: Fri, 28 Nov 2014 12:44:43 -0500 Subject: [PATCH 4/5] short-circuit keys algorithm if no keys changed --- mithril.js | 93 ++++++++++++++++++++++++++++++------------------------ 1 file changed, 52 insertions(+), 41 deletions(-) diff --git a/mithril.js b/mithril.js index 01244e0f..0df14604 100644 --- a/mithril.js +++ b/mithril.js @@ -127,59 +127,70 @@ var m = (function app(window, undefined) { var DELETION = 1, INSERTION = 2 , MOVE = 3; var existing = {}, unkeyed = [], shouldMaintainIdentities = false; for (var i = 0; i < cached.length; i++) { - if (cached[i] && cached[i].attrs && cached[i].attrs.key != null) { + if (cached[i].attrs && cached[i].attrs.key != null) { shouldMaintainIdentities = true; existing[cached[i].attrs.key] = {action: DELETION, index: i} } } if (shouldMaintainIdentities) { - for (var i = 0; i < data.length; i++) { - if (data[i] && data[i].attrs) { - if (data[i].attrs.key != null) { - var key = data[i].attrs.key; - if (!existing[key]) existing[key] = {action: INSERTION, index: i}; - else existing[key] = { - action: MOVE, - index: i, - from: existing[key].index, - element: parentElement.childNodes[existing[key].index] || $document.createElement("div") + var keysDiffer = false + if (data.length != cached.length) keysDiffer = true + else for (var i = 0; i < data.length; i++) { + if (cached[i].attrs && data[i].attrs && cached[i].attrs.key != data[i].attrs.key) { + keysDiffer = true + break + } + } + + if (keysDiffer) { + for (var i = 0; i < data.length; i++) { + if (data[i].attrs) { + if (data[i].attrs.key != null) { + var key = data[i].attrs.key; + if (!existing[key]) existing[key] = {action: INSERTION, index: i}; + else existing[key] = { + action: MOVE, + index: i, + from: existing[key].index, + element: parentElement.childNodes[existing[key].index] || $document.createElement("div") + } } + else unkeyed.push({index: i, element: parentElement.childNodes[i] || $document.createElement("div")}) } - else unkeyed.push({index: i, element: parentElement.childNodes[i] || $document.createElement("div")}) } - } - var actions = [] - for (var prop in existing) actions.push(existing[prop]) - var changes = actions.sort(sortChanges); - var newCached = new Array(cached.length) + var actions = [] + for (var prop in existing) actions.push(existing[prop]) + var changes = actions.sort(sortChanges); + var newCached = new Array(cached.length) - for (var i = 0, change; change = changes[i]; i++) { - if (change.action == DELETION) { - clear(cached[change.index].nodes, cached[change.index]); - newCached.splice(change.index, 1) - } - if (change.action == INSERTION) { - var dummy = $document.createElement("div"); - dummy.key = data[change.index].attrs.key; - parentElement.insertBefore(dummy, parentElement.childNodes[change.index] || null); - newCached.splice(change.index, 0, {attrs: {key: data[change.index].attrs.key}, nodes: [dummy]}) - } - - if (change.action == MOVE) { - if (parentElement.childNodes[change.index] !== change.element && change.element !== null) { - parentElement.insertBefore(change.element, parentElement.childNodes[change.index] || null) + for (var i = 0, change; change = changes[i]; i++) { + if (change.action == DELETION) { + clear(cached[change.index].nodes, cached[change.index]); + newCached.splice(change.index, 1) + } + if (change.action == INSERTION) { + var dummy = $document.createElement("div"); + dummy.key = data[change.index].attrs.key; + parentElement.insertBefore(dummy, parentElement.childNodes[change.index] || null); + newCached.splice(change.index, 0, {attrs: {key: data[change.index].attrs.key}, nodes: [dummy]}) + } + + if (change.action == MOVE) { + if (parentElement.childNodes[change.index] !== change.element && change.element !== null) { + parentElement.insertBefore(change.element, parentElement.childNodes[change.index] || null) + } + newCached[change.index] = cached[change.from] } - newCached[change.index] = cached[change.from] } + for (var i = 0; i < unkeyed.length; i++) { + var change = unkeyed[i]; + parentElement.insertBefore(change.element, parentElement.childNodes[change.index] || null); + newCached[change.index] = cached[change.index] + } + cached = newCached; + cached.nodes = new Array(parentElement.childNodes.length); + for (var i = 0, child; child = parentElement.childNodes[i]; i++) cached.nodes[i] = child } - for (var i = 0; i < unkeyed.length; i++) { - var change = unkeyed[i]; - parentElement.insertBefore(change.element, parentElement.childNodes[change.index] || null); - newCached[change.index] = cached[change.index] - } - cached = newCached; - cached.nodes = new Array(parentElement.childNodes.length); - for (var i = 0, child; child = parentElement.childNodes[i]; i++) cached.nodes[i] = child } //end key algorithm From cbedf999cd305ed88e6b6f5991bc8778c34cb512 Mon Sep 17 00:00:00 2001 From: Leo Horie Date: Fri, 12 Dec 2014 09:30:48 -0500 Subject: [PATCH 5/5] loop refactor --- mithril.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/mithril.js b/mithril.js index 9136d204..25e50ef9 100644 --- a/mithril.js +++ b/mithril.js @@ -134,17 +134,18 @@ var m = (function app(window, undefined) { } } if (shouldMaintainIdentities) { + if (data.indexOf(null) > -1) data = data.filter(function(x) {return x != null}) + var keysDiffer = false if (data.length != cached.length) keysDiffer = true - else for (var i = 0; i < data.length; i++) { - if (cached[i].attrs && data[i].attrs && cached[i].attrs.key != data[i].attrs.key) { + else for (var i = 0, cachedCell, dataCell; cachedCell = cached[i], dataCell = data[i]; i++) { + if (cachedCell.attrs && dataCell.attrs && cachedCell.attrs.key != dataCell.attrs.key) { keysDiffer = true break } } if (keysDiffer) { - if (data.indexOf(null) > -1) data = data.filter(function(x) {return x != null}) for (var i = 0, len = data.length; i < len; i++) { if (data[i] && data[i].attrs) { if (data[i].attrs.key != null) {