1235883 - Add support for inserting lazily loaded children into a DominatorTreeNode tree

Status: RESOLVED FIXED

(DevTools :: Memory, defect)

Description

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

This commit adds the `DominatorTreeNode.insert` method to insert new
DominatorTreeNode children that have just been loaded from the
HeapAnalysesWorker into an existing partially complete DominatorTreeNode
tree. This is done in a persistent and immutable fashion so that we can use
=== to differentiate different generations of `DominatorTreeNode` trees but
still share the vast majority of the underlying structure.

As infrastructure for the insertion, HeapAnalysesWorker's
`getImmediatelyDominated` response also returns the path from the root to the
node whose immediately dominated children are being fetched. This makes it much
easier to know where to insert the newly loaded children.

Comment 1

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Attachment: Add support for inserting lazily loaded children into a DominatorTreeNode tree

Comment 2

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ce56fd3d1c72

Comment 3

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Attachment: Add support for inserting lazily loaded children into a DominatorTreeNode tree

This commit adds the `DominatorTreeNode.insert` method to insert new
DominatorTreeNode children that have just been loaded from the
HeapAnalysesWorker into an existing partially complete DominatorTreeNode
tree. This is done in a persistent and immutable fashion so that we can use
=== to differentiate different generations of `DominatorTreeNode` trees but
still share the vast majority of the underlying structure.

As infrastructure for the insertion, HeapAnalysesWorker's
`getImmediatelyDominated` response also returns the path from the root to the
node whose immediately dominated children are being fetched. This makes it much
easier to know where to insert the newly loaded children.

Comment 4

[Jordan Santell [:jsantell] [@jsantell]]

Comment on attachment 8703030 [details] [diff] [review]
Add support for inserting lazily loaded children into a DominatorTreeNode tree

Review of attachment 8703030 [details] [diff] [review]:
-----------------------------------------------------------------

::: devtools/shared/heapsnapshot/HeapAnalysesWorker.js
@@ +198,5 @@
> +  let id = nodeId;
> +  do {
> +    path.push(id);
> +    id = dominatorTree.getImmediateDominator(id);
> +  } while (id !== null);

I'd wager the memory tool has the most do-while's per LOC

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/e4226ac7285f

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e4226ac7285f

Comment 7

[Mayur Patil]

[bugday-20160323]

Status: RESOLVED,FIXED -> UNVERIFIED

Comments:
STR: Not clear.
Developer specific testing

Component: 
Name			Firefox
Version			46.0b9
Build ID		20160322075646
Update Channel          beta
User Agent		Mozilla/5.0 (Windows NT 6.1; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0
OS			Windows 7 SP1 x86_64

Expected Results: 
Developer specific testing

Actual Results: 
As expected