diff --git a/lib/Document.js b/lib/Document.js index 8f9cbe5..23e301b 100644 --- a/lib/Document.js +++ b/lib/Document.js @@ -58,7 +58,18 @@ function Document(isHTML, address) { // We may need to update this mapping every time a node is rooted // or uprooted, and any time an attribute is added, removed or changed // on a rooted element. - this.byId = Object.create(null); + // + // The mapping is built lazily: it stays null until something actually + // looks an element up by id, and only then is it populated from the + // nodes currently rooted in this document. After that it is kept up to + // date by root()/uproot() and by the id attribute change handler, just as + // it always was. Documents that are never queried by id -- in particular + // the private documents the fragment parser creates and throws away for + // every innerHTML assignment -- no longer pay for an index nobody reads. + // + // A Map keeps ids such as `1023` out of sparse object-index storage, the + // same reason Element uses one for qualified attribute names. + this._byId = null; // This property holds a monotonically increasing value akin to // a timestamp used to record the last modification time of nodes @@ -326,7 +337,9 @@ Document.prototype = Object.create(ContainerNode.prototype, { }}, getElementById: { value: function(id) { - var n = this.byId[id]; + // Coercion can mutate the document, so do it before building the index. + id = String(id); + var n = this._idIndex().get(id); if (!n) return null; if (n instanceof MultiId) { // there was more than one element with this id return n.getFirst(); @@ -336,7 +349,29 @@ Document.prototype = Object.create(ContainerNode.prototype, { _hasMultipleElementsWithId: { value: function(id) { // Used internally by querySelectorAll optimization - return (this.byId[id] instanceof MultiId); + return (this._idIndex().get(id) instanceof MultiId); + }}, + + // The id -> element map, materialized on demand. Everything that reads the + // index goes through here; everything that writes it (addId/delId) leaves it + // alone while it is absent. + _idIndex: { value: function _idIndex() { + var byId = this._byId; + if (byId !== null) return byId; + byId = this._byId = new Map(); + // Populate from the elements rooted in this document right now. _nodes + // is the registry root() and uproot() maintain, so walking it -- rather + // than the tree -- gives exactly the entries the index would hold had it + // been kept up to date all along, and needs no traversal state. + var nodes = this._nodes; + for (var nid = 1, end = this._nextnid; nid < end; nid++) { + var n = nodes[nid]; + if (n !== undefined && n.nodeType === Node.ELEMENT_NODE) { + var id = n.getAttributeNS(null, 'id'); + if (id) this.addId(id, n); + } + } + return byId; }}, // Just copy this method from the Element prototype @@ -631,16 +666,18 @@ Document.prototype = Object.create(ContainerNode.prototype, { // Add a mapping from id to n for n.ownerDocument addId: { value: function addId(id, n) { - var val = this.byId[id]; + var byId = this._byId; + if (byId === null) return; // no index yet; it will pick n up when built + var val = byId.get(id); if (!val) { - this.byId[id] = n; + byId.set(id, n); } else { // TODO: Add a way to opt-out console warnings //console.warn('Duplicate element id ' + id); if (!(val instanceof MultiId)) { val = new MultiId(val); - this.byId[id] = val; + byId.set(id, val); } val.add(n); } @@ -648,17 +685,19 @@ Document.prototype = Object.create(ContainerNode.prototype, { // Delete the mapping from id to n for n.ownerDocument delId: { value: function delId(id, n) { - var val = this.byId[id]; + var byId = this._byId; + if (byId === null) return; // no index yet; nothing to remove n from + var val = byId.get(id); utils.assert(val); if (val instanceof MultiId) { val.del(n); if (val.length === 1) { // convert back to a single node - this.byId[id] = val.downgrade(); + byId.set(id, val.downgrade()); } } else { - this.byId[id] = undefined; + byId.delete(id); } }}, @@ -774,8 +813,10 @@ function root(n) { n.ownerDocument._nodes[n._nid] = n; // Manage id to element mapping if (n.nodeType === Node.ELEMENT_NODE) { - var id = n.getAttribute('id'); - if (id) n.ownerDocument.addId(id, n); + if (n.ownerDocument._byId !== null) { + var id = n.getAttributeNS(null, 'id'); + if (id) n.ownerDocument.addId(id, n); + } // Script elements need to know when they're inserted // into the document @@ -785,8 +826,8 @@ function root(n) { function uproot(n) { // Manage id to element mapping - if (n.nodeType === Node.ELEMENT_NODE) { - var id = n.getAttribute('id'); + if (n.nodeType === Node.ELEMENT_NODE && n.ownerDocument._byId !== null) { + var id = n.getAttributeNS(null, 'id'); if (id) n.ownerDocument.delId(id, n); } n.ownerDocument._nodes[n._nid] = undefined; @@ -829,18 +870,23 @@ function recursivelySetOwner(node, owner) { recursivelySetOwner(kid, owner); } -// A class for storing multiple nodes with the same ID +// A class for storing multiple nodes with the same ID. +// +// A Map keeps the node ids used as keys out of sparse object-index storage: +// a plain object keyed by integers gets an elements backing store sized to +// the largest key, which for node ids means one as large as the document for +// every id that is duplicated. function MultiId(node) { - this.nodes = Object.create(null); - this.nodes[node._nid] = node; + this.nodes = new Map(); + this.nodes.set(node._nid, node); this.length = 1; this.firstNode = undefined; } // Add a node to the list, with O(1) time MultiId.prototype.add = function(node) { - if (!this.nodes[node._nid]) { - this.nodes[node._nid] = node; + if (!this.nodes.has(node._nid)) { + this.nodes.set(node._nid, node); this.length++; this.firstNode = undefined; } @@ -848,8 +894,7 @@ MultiId.prototype.add = function(node) { // Remove a node from the list, with O(1) time MultiId.prototype.del = function(node) { - if (this.nodes[node._nid]) { - delete this.nodes[node._nid]; + if (this.nodes.delete(node._nid)) { this.length--; this.firstNode = undefined; } @@ -861,13 +906,13 @@ MultiId.prototype.del = function(node) { MultiId.prototype.getFirst = function() { /* jshint bitwise: false */ if (!this.firstNode) { - var nid; - for (nid in this.nodes) { - if (this.firstNode === undefined || - this.firstNode.compareDocumentPosition(this.nodes[nid]) & Node.DOCUMENT_POSITION_PRECEDING) { - this.firstNode = this.nodes[nid]; + var self = this; + this.nodes.forEach(function(node) { + if (self.firstNode === undefined || + self.firstNode.compareDocumentPosition(node) & Node.DOCUMENT_POSITION_PRECEDING) { + self.firstNode = node; } - } + }); } return this.firstNode; }; @@ -875,10 +920,7 @@ MultiId.prototype.getFirst = function() { // If there is only one node left, return it. Otherwise return "this". MultiId.prototype.downgrade = function() { if (this.length === 1) { - var nid; - for (nid in this.nodes) { - return this.nodes[nid]; - } + return this.nodes.values().next().value; } return this; }; diff --git a/test/domino.js b/test/domino.js index 00cc051..04d7695 100644 --- a/test/domino.js +++ b/test/domino.js @@ -1984,3 +1984,208 @@ exports.clonedTemplateContentMaintainsHostReference = function () { ); }; +exports.idIndexIsLazy = function() { + // The id -> element index is only built when something actually looks an + // element up by id. Parsing, mutating and serializing a document must not + // build one: the fragment parser creates (and throws away) a private + // document for every innerHTML assignment, and indexing those is pure cost. + var doc = domino.createDocument('
x