Skip to content

Commit

Permalink
Avoid recursion when materializing DOM (Fix meteor#3028)
Browse files Browse the repository at this point in the history 
This way we don't get a stack overflow when materializing nested
Views.  Certain browser/OS combinations seem to have particularly
low budgets (especially Firefox/Windows apparently).

Verified by running https://github.com/mxab/meteor-call-stack-exceed
on Chrome/Mac.  Nesting limit used to be about 160, but now you get
unlimited nesting (tried up to 10,000, which renders in about 7-8
seconds).

Tested for correctness by running all package tests.
  • Loading branch information
@dgreensp
dgreensp committed Mar 26, 2015
1 parent 9872572 commit 28c760e
Show file tree
Hide file tree
Showing 2 changed files with 134 additions and 50 deletions.
77 changes: 57 additions & 20 deletions packages/blaze/materializer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,57 +7,87 @@
// - `intoArray`: the array of DOM nodes and DOMRanges to push the output
// into (required)
// - `parentView`: the View we are materializing content for (optional)
//
// Returns `intoArray`, which is especially useful if you pass in `[]`.
Blaze._materializeDOM = function (htmljs, intoArray, parentView) {
// In order to use fewer stack frames, materializeDOMInner can push
// tasks onto `workStack`, and they will be popped off
// and run, last first, after materializeDOMInner returns. The
// reason we use a stack instead of a queue is so that we recurse
// depth-first, doing newer tasks first.
var workStack = [];
materializeDOMInner(htmljs, intoArray, parentView, workStack);

// A "task" is either an array of arguments to materializeDOM or
// a function to execute. If we only allowed functions as tasks,
// we would have to generate the functions using _.bind or close
// over a loop variable, either of which is a little less efficient.
while (workStack.length) {
// Note that running the workStack task may push new items onto
// the workStack.
var task = workStack.pop();
if (typeof task === 'function') {
task();
} else {
// assume array
materializeDOMInner(task[0], task[1], task[2], workStack);
}
}

return intoArray;
};

var materializeDOMInner = function (htmljs, intoArray, parentView, workStack) {
if (htmljs == null) {
// null or undefined
return intoArray;
return;
}

switch (typeof htmljs) {
case 'string': case 'boolean': case 'number':
intoArray.push(document.createTextNode(String(htmljs)));
return intoArray;
return;
case 'object':
if (htmljs.htmljsType) {
switch (htmljs.htmljsType) {
case HTML.Tag.htmljsType:
intoArray.push(materializeTag(htmljs, parentView));
return intoArray;
intoArray.push(materializeTag(htmljs, parentView, workStack));
return;
case HTML.CharRef.htmljsType:
intoArray.push(document.createTextNode(htmljs.str));
return intoArray;
return;
case HTML.Comment.htmljsType:
intoArray.push(document.createComment(htmljs.sanitizedValue));
return intoArray;
return;
case HTML.Raw.htmljsType:
// Get an array of DOM nodes by using the browser's HTML parser
// (like innerHTML).
var nodes = Blaze._DOMBackend.parseHTML(htmljs.value);
for (var i = 0; i < nodes.length; i++)
intoArray.push(nodes[i]);
return intoArray;
return;
}
} else if (HTML.isArray(htmljs)) {
for (var i = 0; i < htmljs.length; i++) {
Blaze._materializeDOM(htmljs[i], intoArray, parentView);
for (var i = htmljs.length-1; i >= 0; i--) {
workStack.push([htmljs[i], intoArray, parentView]);
}
return intoArray;
return;
} else {
if (htmljs instanceof Blaze.Template) {
htmljs = htmljs.constructView();
// fall through to Blaze.View case below
}
if (htmljs instanceof Blaze.View) {
intoArray.push(Blaze._materializeView(htmljs, parentView));
return intoArray;
Blaze._materializeView(htmljs, parentView, workStack, intoArray);
return;
}
}
}

throw new Error("Unexpected object in htmljs: " + htmljs);
};

var materializeTag = function (tag, parentView) {
var materializeTag = function (tag, parentView, workStack) {
var tagName = tag.tagName;
var elem;
if ((HTML.isKnownSVGElement(tagName) || isSVGAnchor(tag))
Expand Down Expand Up @@ -118,13 +148,20 @@ var materializeTag = function (tag, parentView) {
});
}

var childNodesAndRanges = Blaze._materializeDOM(children, [], parentView);
for (var i = 0; i < childNodesAndRanges.length; i++) {
var x = childNodesAndRanges[i];
if (x instanceof Blaze._DOMRange)
x.attach(elem);
else
elem.appendChild(x);
if (children.length) {
var childNodesAndRanges = [];
// push this function first so that it's done last
workStack.push(function () {
for (var i = 0; i < childNodesAndRanges.length; i++) {
var x = childNodesAndRanges[i];
if (x instanceof Blaze._DOMRange)
x.attach(elem);
else
elem.appendChild(x);
}
});
// now push the task that calculates childNodesAndRanges
workStack.push([children, childNodesAndRanges, parentView]);
}

return elem;
Expand Down
107 changes: 77 additions & 30 deletions packages/blaze/view.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,46 @@ Blaze._createView = function (view, parentView, forExpansion) {
Blaze._fireCallbacks(view, 'created');
};

Blaze._materializeView = function (view, parentView) {
var doFirstRender = function (view, initialContent) {
var domrange = new Blaze._DOMRange(initialContent);
view._domrange = domrange;
domrange.view = view;
view.isRendered = true;
Blaze._fireCallbacks(view, 'rendered');

var teardownHook = null;

domrange.onAttached(function attached(range, element) {
view._isAttached = true;

teardownHook = Blaze._DOMBackend.Teardown.onElementTeardown(
element, function teardown() {
Blaze._destroyView(view, true /* _skipNodes */);
});
});

// tear down the teardown hook
view.onViewDestroyed(function () {
teardownHook && teardownHook.stop();
teardownHook = null;
});

return domrange;
};

// Take an uncreated View `view` and create and render it to DOM,
// setting up the autorun that updates the View. Returns a new
// DOMRange, which has been associated with the View.
//
// The private arguments `_workStack` and `_intoArray` are passed in
// by Blaze._materializeDOM. If provided, then we avoid the mutual
// recursion of calling back into Blaze._materializeDOM so that deep
// View hierarchies don't blow the stack. Instead, we push tasks onto
// workStack for the initial rendering and subsequent setup of the
// View, and they are done after we return. When there is a
// _workStack, we do not return the new DOMRange, but instead push it
// into _intoArray from a _workStack task.
Blaze._materializeView = function (view, parentView, _workStack, _intoArray) {
Blaze._createView(view, parentView);

var domrange;
Expand All @@ -298,20 +337,16 @@ Blaze._materializeView = function (view, parentView) {
var htmljs = view._render();
view._isInRender = false;

Tracker.nonreactive(function doMaterialize() {
var rangesAndNodes = Blaze._materializeDOM(htmljs, [], view);
if (c.firstRun || ! Blaze._isContentEqual(lastHtmljs, htmljs)) {
if (c.firstRun) {
domrange = new Blaze._DOMRange(rangesAndNodes);
view._domrange = domrange;
domrange.view = view;
view.isRendered = true;
} else {
if (! c.firstRun) {
Tracker.nonreactive(function doMaterialize() {
// re-render
var rangesAndNodes = Blaze._materializeDOM(htmljs, [], view);
if (! Blaze._isContentEqual(lastHtmljs, htmljs)) {
domrange.setMembers(rangesAndNodes);
Blaze._fireCallbacks(view, 'rendered');
}
Blaze._fireCallbacks(view, 'rendered');
}
});
});
}
lastHtmljs = htmljs;

// Causes any nested views to stop immediately, not when we call
Expand All @@ -323,25 +358,37 @@ Blaze._materializeView = function (view, parentView) {
});
}, undefined, 'materialize');

var teardownHook = null;

domrange.onAttached(function attached(range, element) {
view._isAttached = true;

teardownHook = Blaze._DOMBackend.Teardown.onElementTeardown(
element, function teardown() {
Blaze._destroyView(view, true /* _skipNodes */);
});
});

// tear down the teardown hook
view.onViewDestroyed(function () {
teardownHook && teardownHook.stop();
teardownHook = null;
});
// first render. lastHtmljs is the first htmljs.
var initialContents;
if (! _workStack) {
initialContents = Blaze._materializeDOM(lastHtmljs, [], view);
domrange = doFirstRender(view, initialContents);
initialContents = null; // help GC because we close over this scope a lot
} else {
// We're being called from Blaze._materializeDOM, so to avoid
// recursion and save stack space, provide a description of the
// work to be done instead of doing it. Tasks pushed onto
// _workStack will be done in LIFO order after we return.
// The work will still be done within a Tracker.nonreactive,
// because it will be done by some call to Blaze._materializeDOM
// (which is always called in a Tracker.nonreactive).
initialContents = [];
// push this function first so that it happens last
_workStack.push(function () {
domrange = doFirstRender(view, initialContents);
initialContents = null; // help GC because of all the closures here
_intoArray.push(domrange);
});
// now push the task that calculates initialContents
_workStack.push([lastHtmljs, initialContents, view]);
}
});

return domrange;
if (! _workStack) {
return domrange;
} else {
return null;
}
};

// Expands a View to HTMLjs, calling `render` recursively on all
Expand Down

0 comments on commit 28c760e

Please sign in to comment.