New issue
Advanced search Search tips

Issue 808035 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocked on:
issue 811842



Sign in to add a comment

Remove special handling of user roots in Heap Snapshot

Project Member Reported by u...@chromium.org, Feb 1 2018

Issue description

The heap snapshot worker tries to give preference to the retaining paths from the user roots.

It is implemented by doing two BFS traversals:

    this.forEachRoot(enqueueNode.bind(null, 1), true);
    this._bfs(nodesToVisit, nodesToVisitLength, distances, filter);

    // bfs for the rest of objects
    nodesToVisitLength = 0;
    this.forEachRoot(enqueueNode.bind(null, HeapSnapshotModel.baseSystemDistance), false);
    this._bfs(nodesToVisit, nodesToVisitLength, distances, filter);

The HeapSnapshotModel.baseSystemDistance is a sentinel big number (~infinity).

This works if an object is retained by a user root.

If the object is not retained by a user root, then its distance appears as unknown.
See attached screenshot.

I propose to remove special handling of user roots (and GC roots) and just do a single BFS from the global root node:
https://chromium-review.googlesource.com/c/chromium/src/+/897324

This together with the upcoming DOM node snapshotting will ensure that each object has a known distance.







 
user-roots.png
38.3 KB View Download

Comment 1 by u...@chromium.org, Feb 13 2018

Blockedon: 811842
In offline discussion alph@ mentioned that we need special handling for two reasons:
1) To avoid confusion if the object is retained by DevTools console via strong global handle.
2) To make Containment View navigation simpler.

I think 1) can be solved by annotating global handles, so that retaining path via console becomes obvious to the user (the user can then clear console).

I don't have good solution for 2). We need to check if Containment View is used often.




Cc: mlippautz@chromium.org
Ping. Can we discuss this one again? 

We actually have annotations by now (and are adding more and more) and they help digging down several issues.

I just got puzzled again yesterday by having the recursion for native contexts (see screenshot). I get that users might consider this as roots but what does it actually mean to have a recursion in the retaining path? This could also be interpreted as being prone to cycles, as an object is keeping itself alive.

I locally commented out the user root stuff and imho got a really clean picture where everything was the somehow retained by actual GC roots. 
Screen Shot 2018-03-15 at 11.01.16 AM.png
701 KB View Download
What's the status of this?

Sign in to add a comment