[webkit-reviews] review granted: [Bug 196890] Web Inspector: Heap: don't use recursion when calculating root paths : [Attachment 367366] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Apr 15 11:18:04 PDT 2019

Joseph Pecoraro <joepeck at webkit.org> has granted Devin Rousso
<drousso at apple.com>'s request for review:
Bug 196890: Web Inspector: Heap: don't use recursion when calculating root

Attachment 367366: Patch


--- Comment #3 from Joseph Pecoraro <joepeck at webkit.org> ---
Comment on attachment 367366
  --> https://bugs.webkit.org/attachment.cgi?id=367366

View in context: https://bugs.webkit.org/attachment.cgi?id=367366&action=review


> Source/WebInspectorUI/UserInterface/Workers/HeapSnapshot/HeapSnapshot.js:756
> +	   for (let i = 0; i < stack.length; ++i) {
> +	       let {currentPath, nodeOrdinal} = stack[i];

This list is not being treated as a stack. In fact it is always growing because
nothing nothing is ever removed.

Normally when we have a stack approach like this we process it to exhaustion
like this:

    let stack = [{...entry...}];
    while (stack.length) {
	let entry = stack.pop();
	// Process entry, maybe push new stack entries.

Which I think could be done here as long as you convert these two lines to:

    while (stack.length) {
	let {currentPath, nodeOridinal} = stack.pop();

That said, items are going to be processes on the stack in reverse order... so
to get unchanging output you could iterate the edges in forward order such as:

    while (stack.length) {
	let {currentPath, nodeOridinal} = stack.unshift();

That should get you output that is consistent with the current code.

More information about the webkit-reviews mailing list