Skip to content

Commit

Permalink
Web Worker improvements and fixes
Browse files Browse the repository at this point in the history
- Do not clean up the documentElement in enableProllyfill if it is
  currently being rendered.
- Moved Worker event handlers to before postMessage is called.
  • Loading branch information
tbranyen committed Jan 8, 2016
1 parent e54882b commit c5b721b
Show file tree
Hide file tree
Showing 7 changed files with 49 additions and 21 deletions.
34 changes: 24 additions & 10 deletions dist/diffhtml.js
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,8 @@ var _transitions = _dereq_('./transitions');

var _custom = _dereq_('./element/custom');

var _tree = _dereq_('./node/tree');

function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; }

/**
Expand Down Expand Up @@ -550,8 +552,12 @@ function enableProllyfill() {

// After the initial render, clean up the resources, no point in lingering.
documentElement.addEventListener('renderComplete', function render() {
var elementMeta = _tree.TreeCache.get(documentElement) || {};

// Release resources allocated to the element.
documentElement.diffRelease(documentElement);
if (!elementMeta.isRendering) {
documentElement.diffRelease(documentElement);
}

// Remove this event listener.
documentElement.removeEventListener('renderComplete', render);
Expand All @@ -574,7 +580,7 @@ function enableProllyfill() {
}
}

},{"./element/custom":1,"./errors":4,"./node/patch":7,"./node/release":8,"./transitions":13}],6:[function(_dereq_,module,exports){
},{"./element/custom":1,"./errors":4,"./node/patch":7,"./node/release":8,"./node/tree":10,"./transitions":13}],6:[function(_dereq_,module,exports){
'use strict';

Object.defineProperty(exports, "__esModule", {
Expand Down Expand Up @@ -858,13 +864,13 @@ function patchNode(element, newHTML, options) {
if (typeof newHTML !== 'string') {
transferObject.newTree = (0, _make2.default)(newHTML);

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

// Transfer this buffer to the worker, which will take over and process the
// markup.
worker.postMessage(transferObject);

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

return;
}

Expand All @@ -875,12 +881,12 @@ 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 = (0, _render.completeWorkerRender)(element, elementMeta);

// Transfer this buffer to the worker, which will take over and process the
// markup.
worker.postMessage(transferObject);

// Wait for the worker to finish processing and then apply the patchset.
worker.onmessage = (0, _render.completeWorkerRender)(element, elementMeta);
} else {
// We're rendering in the UI thread.
elementMeta.isRendering = true;
Expand Down Expand Up @@ -951,10 +957,14 @@ function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { de
function releaseNode(element) {
var elementMeta = _tree.TreeCache.get(element);

// Do not remove an element that is in process of rendering. User intentions
// come first, so if we are cleaning up an element being used by another part
// of the code, keep it alive.
if (elementMeta) {
// If there is a worker associated with this element, then kill it.
if (elementMeta.worker) {
elementMeta.worker.terminate();
delete elementMeta.worker;
}

// If there was a tree set up, recycle the memory allocated for it.
Expand Down Expand Up @@ -1488,7 +1498,11 @@ function process(element, patches) {
}

// Append the element first, before doing the replacement.
patch.old.parentNode.insertBefore(patch.new, patch.old.nextSibling);
if (patch.old.nextSibling) {
patch.old.parentNode.insertBefore(patch.new, patch.old.nextSibling);
} else {
patch.old.parentNode.appendChild(patch.new);
}

// Removed state for transitions API.
var allPromises = [];
Expand Down Expand Up @@ -2623,7 +2637,7 @@ function startup(worker) {
syncNode(oldTree, newTree, patches);

// Protect the current `oldTree` so that no Nodes will be accidentally
// recycled in the
// recycled in the cleanup process.
protectElement(oldTree);

// Send the patches back to the userland.
Expand Down
7 changes: 6 additions & 1 deletion lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import patchNode from './node/patch';
import releaseNode from './node/release';
import { states } from './transitions';
import { components } from './element/custom';
import { TreeCache } from './node/tree';

import { TransitionStateError, DOMException } from './errors';
// Export the custom Error constructors so that instanceof checks can be made
Expand Down Expand Up @@ -288,8 +289,12 @@ export function enableProllyfill() {

// After the initial render, clean up the resources, no point in lingering.
documentElement.addEventListener('renderComplete', function render() {
var elementMeta = TreeCache.get(documentElement) || {};

// Release resources allocated to the element.
documentElement.diffRelease(documentElement);
if (!elementMeta.isRendering) {
documentElement.diffRelease(documentElement);
}

// Remove this event listener.
documentElement.removeEventListener('renderComplete', render);
Expand Down
13 changes: 7 additions & 6 deletions lib/node/patch.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ export default function patchNode(element, newHTML, options) {
return elementMeta.renderBuffer;
}


// 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.
Expand Down Expand Up @@ -138,13 +139,13 @@ export default function patchNode(element, newHTML, options) {
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);

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

return;
}

Expand All @@ -155,12 +156,12 @@ 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);

// Wait for the worker to finish processing and then apply the patchset.
worker.onmessage = completeWorkerRender(element, elementMeta);
}
else {
// We're rendering in the UI thread.
Expand Down
4 changes: 4 additions & 0 deletions lib/node/release.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,14 @@ import makeNode from './make';
export default function releaseNode(element) {
let elementMeta = TreeCache.get(element);

// Do not remove an element that is in process of rendering. User intentions
// come first, so if we are cleaning up an element being used by another part
// of the code, keep it alive.
if (elementMeta) {
// If there is a worker associated with this element, then kill it.
if (elementMeta.worker) {
elementMeta.worker.terminate();
delete elementMeta.worker;
}

// If there was a tree set up, recycle the memory allocated for it.
Expand Down
8 changes: 6 additions & 2 deletions lib/patches/process.js
Original file line number Diff line number Diff line change
Expand Up @@ -185,9 +185,13 @@ export default function process(element, patches) {
'document root?');
}


// Append the element first, before doing the replacement.
patch.old.parentNode.insertBefore(patch.new, patch.old.nextSibling);
if (patch.old.nextSibling) {
patch.old.parentNode.insertBefore(patch.new, patch.old.nextSibling);
}
else {
patch.old.parentNode.appendChild(patch.new);
}

// Removed state for transitions API.
let allPromises = [];
Expand Down
2 changes: 1 addition & 1 deletion lib/worker/source.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ export default function startup(worker) {
syncNode(oldTree, newTree, patches);

// Protect the current `oldTree` so that no Nodes will be accidentally
// recycled in the
// recycled in the cleanup process.
protectElement(oldTree);

// Send the patches back to the userland.
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"scripts": {
"start": "http-server",
"build": "browserify -t [ babelify --presets [ es2015 ] ] -s diff lib/index.js | derequire > dist/diffhtml.js",
"watch": "watchify -t [ babelify --presets [ es2015 ] ] -s diff lib/index.js -o dist/diffhtml.js -v",
"watch": "watchify -t [ babelify --presets [ es2015 ] ] -s diff lib/index.js -o 'derequire > dist/diffhtml.js' -v",
"jshint": "jshint lib/**/*.js",
"test": "npm run jshint && karma start --single-run"
},
Expand Down

0 comments on commit c5b721b

Please sign in to comment.