Skip to content

Commit

Permalink
Fixes that improve the use of Workers and memory
Browse files Browse the repository at this point in the history
Now aserting our internal allocations per test to ensure we aren't
causing leaks.

- Allow only render to occur at a time

  Prevents conflicts and makes diffHTML more transactional.

- Validate memory per test, fixes rentention bugs

  This ensures that we properly clean memory for every render scenario test.
  • Loading branch information
tbranyen committed Jan 24, 2016
1 parent ec1feee commit cdaf91e
Show file tree
Hide file tree
Showing 25 changed files with 704 additions and 542 deletions.
606 changes: 330 additions & 276 deletions dist/diffhtml.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion karma.conf.js
Expand Up @@ -9,7 +9,7 @@ module.exports = function(config) {
files: [
'node_modules/promise-polyfill/Promise.js',
'node_modules/phantomjs-polyfill/bind-polyfill.js',
'node_modules/weakmap/weakmap.js',
'node_modules/es6-collections/es6-collections.js',
'lib/**/*.js',
'test/assert.js',

Expand Down
17 changes: 17 additions & 0 deletions lib/index.js
Expand Up @@ -259,6 +259,7 @@ export function enableProllyfill() {
let copy = {
set: function(val) {
val = Object.keys(val).length ? val : Object.getPrototypeOf(val);

for (let key in val) {
if (val.hasOwnProperty(key)) {
this[key] = val[key];
Expand Down Expand Up @@ -286,6 +287,22 @@ export function enableProllyfill() {
*/
let activateComponents = function() {
var documentElement = document.documentElement;
var bufferSet = false;

// If this element is already rendering, add this new render request into
// the buffer queue. Check and see if any element is currently rendering,
// can only do one at a time.
TreeCache.forEach(function(elementMeta, element) {
if (elementMeta.isRendering) {
bufferSet = true;
}
});

// Short circuit the rest of this render.
if (bufferSet) {
// Remove the load event listener, since it's complete.
return window.removeEventListener('load', activateComponents);
}

// After the initial render, clean up the resources, no point in lingering.
documentElement.addEventListener('renderComplete', function render() {
Expand Down
46 changes: 15 additions & 31 deletions lib/node/make.js
@@ -1,9 +1,7 @@
import { pools as _pools } from '../util/pools';
import { protectElement as _protectElement, } from '../util/memory';
import { components, upgrade } from '../element/custom';

var pools = _pools;
var protectElement = _protectElement;
var empty = {};

// Cache created nodes inside this object.
Expand All @@ -13,10 +11,9 @@ make.nodes = {};
* Converts a live node into a virtual node.
*
* @param node
* @param protect
* @return
*/
export default function make(node, protect) {
export default function make(node) {
// Default the nodeType to Element (1).
let nodeType = 'nodeType' in node ? node.nodeType : 1;

Expand All @@ -29,25 +26,20 @@ export default function make(node, protect) {
// diff and patch.
let entry = pools.elementObject.get();

// Associate this newly allocated uuid with this Node.
make.nodes[entry.uuid] = node;

// Set a lowercased (normalized) version of the element's nodeName.
entry.nodeName = node.nodeName.toLowerCase();

// If the element is a text node set the nodeValue.
if (nodeType === 3) {
let nodeValue = node.textContent;
entry.nodeValue = nodeValue;
entry.nodeValue = node.textContent;
}

entry.childNodes.length = 0;
entry.attributes.length = 0;

if (protect) {
// Add to internal lookup.
make.nodes[entry.uuid] = node;

// Ensure this element will not be automatically cleaned up.
protectElement(entry);
}

// Collect attributes.
let attributes = node.attributes;

Expand All @@ -59,10 +51,6 @@ export default function make(node, protect) {
for (let i = 0; i < attributesLength; i++) {
let attr = pools.attributeObject.get();

if (protect) {
pools.attributeObject.protect(attr);
}

attr.name = attributes[i].name;
attr.value = attributes[i].value;

Expand All @@ -78,26 +66,22 @@ export default function make(node, protect) {
// If the element has child nodes, convert them all to virtual nodes.
if (node.nodeType !== 3 && childNodes) {
for (let i = 0; i < childNodesLength; i++) {
let newNode = make(childNodes[i], protect);
let newNode = make(childNodes[i]);

if (newNode) {
entry.childNodes[entry.childNodes.length] = newNode;
}
}
}

// TODO Rename this to first-run, because we're calling the attach callback
// and protecting now.
if (protect) {
if (components[entry.nodeName]) {
// Reset the prototype chain for this element. Upgrade will return `true`
// if the element was upgraded for the first time. This is useful so we
// don't end up in a loop when working with the same element.
if (upgrade(entry.nodeName, node)) {
// If the Node is in the DOM, trigger attached callback.
if (node.parentNode && node.attachedCallback) {
node.attachedCallback();
}
if (components[entry.nodeName]) {
// Reset the prototype chain for this element. Upgrade will return `true`
// if the element was upgraded for the first time. This is useful so we
// don't end up in a loop when working with the same element.
if (upgrade(entry.nodeName, node)) {
// If the Node is in the DOM, trigger attached callback.
if (node.parentNode && node.attachedCallback) {
node.attachedCallback();
}
}
}
Expand Down
164 changes: 82 additions & 82 deletions lib/node/patch.js
@@ -1,45 +1,12 @@
import CustomEvent from 'custom-event';
import { create as createWorker, hasWorker } from '../worker/create';
import { cleanMemory, protectElement, unprotectElement } from '../util/memory';
import { parseHTML } from '../util/parser';
import { completeRender } from '../util/render';
import makeNode from './make';
import processPatches from '../patches/process';
import syncNode from './sync';
import { TreeCache } from './tree';
import { completeWorkerRender } from '../worker/render';

/**
* When the UI thread completes, clean up memory and schedule the next render
* if necessary.
*
* @param element
* @param elementMeta
*/
function completeUIRender(element, elementMeta) {
return function() {
// Mark that this element has initially rendered and is done rendering.
elementMeta.isRendering = false;

// Set the innerHTML.
elementMeta._innerHTML = element.innerHTML;
elementMeta._outerHTML = element.outerHTML;
elementMeta._textContent = element.textContent;

cleanMemory();

// Dispatch an event on the element once rendering has completed.
element.dispatchEvent(new CustomEvent('renderComplete'));

// TODO Update this comment and/or refactor to use the same as the Worker.
if (elementMeta.renderBuffer) {
let nextRender = elementMeta.renderBuffer;
elementMeta.renderBuffer = undefined;

// Noticing some weird performance implications with this concept.
patchNode(element, nextRender.newHTML, nextRender.options);
}
};
}
import { pools } from '../util/pools';

/**
* Patches an element's DOM to match that of the passed markup.
Expand All @@ -56,30 +23,44 @@ export default function patchNode(element, newHTML, options) {
// The element meta object is a location to associate metadata with the
// currently rendering element. This prevents attaching properties to the
// instance itself.
var elementMeta = TreeCache.get(element) || {
// Store protected elements here from the Worker.
workerCache: []
};
var elementMeta = TreeCache.get(element) || {};

// Last options used.
elementMeta.options = options;

// Always ensure the most up-to-date meta object is stored.
TreeCache.set(element, elementMeta);

var bufferSet = false;
var isInner = options.inner;
var previousMarkup = elementMeta.previousMarkup;

// If this element is already rendering, add this new render request into the
// buffer queue.
if (elementMeta.isRendering) {
elementMeta.renderBuffer = { newHTML, options };
return elementMeta.renderBuffer;
}
// buffer queue. Check and see if any element is currently rendering, can
// only do one at a time.
TreeCache.forEach(function(_elementMeta, _element) {
if (_elementMeta.isRendering) {
elementMeta.renderBuffer = { element, newHTML, options };
bufferSet = true;
}
});

// Short circuit the rest of this render.
if (bufferSet) { return; }

// If the operation is `innerHTML`, but the contents haven't changed.
var sameInnerHTML = options.inner && element.innerHTML === newHTML;
// If the operation is `outerHTML`, but the contents haven't changed.
var sameOuterHTML = !options.inner && element.outerHTML === newHTML;
var sameInnerHTML = isInner ? previousMarkup === element.innerHTML : true;
var sameOuterHTML = !isInner ? previousMarkup === element.outerHTML : true;
var sameTextContent = elementMeta._textContent === element.textContent;

// If the contents haven't changed, abort, since there is no point in
// continuing.
if ((sameInnerHTML || sameOuterHTML) && elementMeta.oldTree) { return; }
if (elementMeta.newHTML === newHTML) {
elementMeta.isRendering = false;
return;
}

// Associate the last markup rendered with this element.
elementMeta.newHTML = newHTML;

// Start with worker being a falsy value.
var worker = null;
Expand All @@ -90,30 +71,26 @@ export default function patchNode(element, newHTML, options) {
worker = elementMeta.worker = elementMeta.worker || createWorker();
}

if (
// If the operation is `innerHTML`, and the current element's contents have
// changed since the last render loop, recalculate the tree.
(options.inner && elementMeta._innerHTML !== element.innerHTML) ||

// If the operation is `outerHTML`, and the current element's contents have
// changed since the last render loop, recalculate the tree.
(!options.inner && elementMeta._outerHTML !== element.outerHTML) ||

// If the text content ever changes, recalculate the tree.
(elementMeta._textContent !== element.textContent) ||

// The last render was done via Worker, but now we're rendering in the UI
// thread. This is very uncommon, but we need to ensure tree's stay in
// sync.
(elementMeta.renderedViaWorker === true && !options.enableWorker)
) {
if (elementMeta.oldTree) {
unprotectElement(elementMeta.oldTree, makeNode);
cleanMemory();
var rebuildTree = function() {
var oldTree = elementMeta.oldTree;

if (oldTree) {
unprotectElement(oldTree, makeNode);
cleanMemory(makeNode);
}

elementMeta.oldTree = makeNode(element, true);
elementMeta.oldTree = protectElement(makeNode(element));
elementMeta.updateWorkerTree = true;
};

// The last render was done via Worker, but now we're rendering in the UI
// thread. This is very uncommon, but we need to ensure trees stay in sync.
if (elementMeta.renderedViaWorker === true && !options.enableWorker) {
rebuildTree();
}

if (!sameInnerHTML || !sameOuterHTML || !sameTextContent) {
rebuildTree();
}

// Will want to ensure that the first render went through, the worker can
Expand All @@ -122,6 +99,7 @@ export default function patchNode(element, newHTML, options) {
// Set a render lock as to not flood the worker.
elementMeta.isRendering = true;
elementMeta.renderedViaWorker = true;
elementMeta.workerCache = elementMeta.workerCache || [];

// Attach all properties here to transport.
let transferObject = {};
Expand All @@ -133,15 +111,30 @@ export default function patchNode(element, newHTML, options) {
elementMeta.updateWorkerTree = false;
}

// Attach the parent element's uuid.
transferObject.uuid = elementMeta.oldTree.uuid;
// Wait for the worker to finish processing and then apply the patchset.
worker.onmessage = function(ev) {
var wrapRender = cb => () => {
elementMeta.hasWorkerRendered = true;
cb();
};
// Wait until all promises have resolved, before finishing up the patch
// cycle.
// Process the data immediately and wait until all transition callbacks
// have completed.
var processPromise = processPatches(element, ev.data.patches);
var invokeRender = wrapRender(completeRender(element, elementMeta));

// Operate synchronously unless opted into a Promise-chain.
if (processPromise) {
processPromise.then(invokeRender).catch(ex => console.log(ex));
} else {
invokeRender();
}
};

if (typeof newHTML !== 'string') {
transferObject.newTree = makeNode(newHTML);

// Wait for the worker to finish processing and then apply the patchset.
worker.onmessage = completeWorkerRender(element, elementMeta);

// Transfer this buffer to the worker, which will take over and process the
// markup.
worker.postMessage(transferObject);
Expand All @@ -156,14 +149,20 @@ export default function patchNode(element, newHTML, options) {
// Add properties to send to worker.
transferObject.isInner = options.inner;

// Wait for the worker to finish processing and then apply the patchset.
worker.onmessage = completeWorkerRender(element, elementMeta);

// Transfer this buffer to the worker, which will take over and process the
// markup.
worker.postMessage(transferObject);
}
else {
if (elementMeta.renderedViaWorker && elementMeta.oldTree) {
rebuildTree();
}

if (elementMeta.workerCache) {
elementMeta.workerCache.forEach(x => unprotectElement(x, makeNode));
delete elementMeta.workerCache;
}

// We're rendering in the UI thread.
elementMeta.isRendering = true;

Expand All @@ -177,11 +176,11 @@ export default function patchNode(element, newHTML, options) {
newTree = parseHTML(newHTML, options.inner);
}
else {
newTree = options.inner ? [makeNode(newHTML)] : makeNode(newHTML);
newTree = makeNode(newHTML);
}

if (options.inner) {
let childNodes = newTree;
let childNodes = Array.isArray(newTree) ? newTree : [newTree];

newTree = {
childNodes,
Expand All @@ -194,17 +193,18 @@ export default function patchNode(element, newHTML, options) {

// Synchronize the tree.
let patches = syncNode(elementMeta.oldTree, newTree);
let invokeRender = completeRender(element, elementMeta);

// Process the data immediately and wait until all transition callbacks
// have completed.
let processPromise = processPatches(element, patches);

// Operate synchronously unless opted into a Promise-chain.
if (processPromise) {
processPromise.then(completeUIRender(element, elementMeta));
processPromise.then(invokeRender).catch(ex => console.log(ex));
}
else {
completeUIRender(element, elementMeta)();
invokeRender();
}
}
}

0 comments on commit cdaf91e

Please sign in to comment.