Commit 786ed1e0 authored by Shuotao Gao's avatar Shuotao Gao Committed by Commit Bot

Revert "Reland "[heap-profiler] Merge DOM node and its JS wrapper node in heap snapshot.""

This reverts commit 92aa9f81.

Reason for revert: 
inspector-protocol/heap-profiler/heap-snapshot-merged-nodes.js is failing since https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Linux%20Trusty%20MSAN/6477 until https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Linux%20Trusty%20MSAN/6482 when this revert is created.

Original change's description:
> Reland "[heap-profiler] Merge DOM node and its JS wrapper node in heap snapshot."
> 
> This relands commit d690c638.
> 
> Original change's description:
> > [heap-profiler] Merge DOM node and its JS wrapper node in heap snapshot.
> >
> > Each DOM node has a corresponding JS wrapper node. In heap snapshot
> > they appear as duplicates.
> >
> > Example retaining path with duplicates:
> > - [1] in InternalNode @2041178784
> > - [1] in HTMLDivElement @2041079168  // DOM node
> > - [3] in HTMLDivElement @2231// JS wrapper
> > - retainer in Window / @2105
> >
> > The heap snapshot generator now can merge an embedder node with a node
> > returned by EmbedderGraph::Node::WrapperNode() function.
> >
> > This patch implements the WrapperNode() function for each DOM node.
> >
> > Bug: chromium:811925
> > Change-Id: I9ea6bc7e45f8ab3d54828b9ac61f7230d04d8019
> > Reviewed-on: https://chromium-review.googlesource.com/928503
> > Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
> > Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#539132}
> 
> 
> Change-Id: I4223d1369874ad34dfff1a74d5725360c360300c
> Reviewed-on: https://chromium-review.googlesource.com/951252
> Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#541403}

TBR=ulan@chromium.org,haraken@chromium.org,hbos@chromium.org

Change-Id: Ic99975f4052831b7a799c882a2227e1a64d04f97
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/953368Reviewed-by: default avatarShuotao Gao <stgao@chromium.org>
Commit-Queue: Shuotao Gao <stgao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541585}
parent 251e255f
Test that DOM node and its JS wrapper appear as a single node.
Took heap snapshot
Parsed snapshot
SUCCESS: found leaking
SUCCESS: retaining path = [EventListener, InternalNode, HTMLDivElement, Window / file://, ]
(async function(testRunner) {
var {page, session, dp} = await testRunner.startBlank(
`Test that DOM node and its JS wrapper appear as a single node.`);
await session.evaluate(`
var retainer = null;
function run() {
function leaking() {
console.log('leaking');
}
var div = document.createElement('div');
div.addEventListener('click', leaking, true);
retainer = div;
}
run();
`);
var Helper = await testRunner.loadScript('resources/heap-snapshot-common.js');
var helper = await Helper(testRunner, session);
var snapshot = await helper.takeHeapSnapshot();
var node;
for (var it = snapshot._allNodes(); it.hasNext(); it.next()) {
if (it.node.type() === 'closure' && it.node.name() === 'leaking') {
node = it.node;
break;
}
}
if (node)
testRunner.log('SUCCESS: found ' + node.name());
else
return testRunner.fail('cannot find leaking node');
var retainers = helper.firstRetainingPath(node).map(node => node.name());
var actual = retainers.join(', ');
testRunner.log(`SUCCESS: retaining path = [${actual}]`);
testRunner.completeTest();
})
......@@ -2,5 +2,5 @@ Test retaining path for an event listener.
Took heap snapshot
Parsed snapshot
SUCCESS: found myEventListener
SUCCESS: retaining path = [EventListener, InternalNode, HTMLBodyElement, HTMLHtmlElement, HTMLDocument, Window / file://, ]
SUCCESS: retaining path = [EventListener, InternalNode, HTMLBodyElement, HTMLHtmlElement, HTMLDocument]
......@@ -29,6 +29,9 @@
return testRunner.fail('cannot find myEventListener node');
var retainers = helper.firstRetainingPath(node).map(node => node.name());
// Limit to the retainers until the Window object to keep the test robust
// against root node name changes.
retainers = retainers.slice(0, retainers.indexOf('Window'));
var actual = retainers.join(', ');
testRunner.log(`SUCCESS: retaining path = [${actual}]`);
testRunner.completeTest();
......
......@@ -4,6 +4,6 @@ Parsed snapshot
SUCCESS: found leaking
SUCCESS: immediate retainer is EventListener.
SUCCESS: found multiple retaining paths.
SUCCESS: path1 = [HTMLBodyElement, HTMLHtmlElement, HTMLDocument, Window / file://, ]
SUCCESS: path2 = [HTMLDivElement, HTMLBodyElement, HTMLHtmlElement, HTMLDocument, Window / file://, ]
SUCCESS: path1 = [HTMLBodyElement, HTMLHtmlElement, HTMLDocument]
SUCCESS: path2 = [HTMLDivElement, HTMLBodyElement, HTMLHtmlElement, HTMLDocument]
......@@ -43,6 +43,9 @@
for (var iter = eventListener.retainers(); iter.hasNext(); iter.next()) {
var path = helper.firstRetainingPath(iter.retainer.node());
path = path.map(node => node.name());
// Limit the path until the Window object to keep the test robust
// against root node name changes.
path = path.slice(0, path.indexOf('Window'));
retainingPaths.push(path.join(', '));
}
......
......@@ -40,9 +40,10 @@ void V8EmbedderGraphBuilder::VisitPersistentHandle(
isolate_, v8::Persistent<v8::Object>::Cast(*value));
ScriptWrappable* traceable = ToScriptWrappable(v8_value);
if (traceable) {
Graph::Node* wrapper = GraphNode(v8_value);
// Add v8_value => traceable edge.
Graph::Node* graph_node =
GraphNode(traceable, traceable->NameInHeapSnapshot(), wrapper);
GraphNode(traceable, traceable->NameInHeapSnapshot());
graph_->AddEdge(GraphNode(v8_value), graph_node);
// Visit traceable members. This will also add traceable => v8_value edge.
ParentScope parent(this, graph_node);
traceable->TraceWrappers(this);
......@@ -64,8 +65,8 @@ void V8EmbedderGraphBuilder::Visit(
// Add an edge from the current parent to this object.
// Also push the object to the worklist in order to process its members.
const void* traceable = wrapper_descriptor.base_object_payload;
Graph::Node* graph_node = GraphNode(
traceable, wrapper_descriptor.name_callback(traceable), nullptr);
Graph::Node* graph_node =
GraphNode(traceable, wrapper_descriptor.name_callback(traceable));
graph_->AddEdge(current_parent_, graph_node);
if (!visited_.Contains(traceable)) {
visited_.insert(traceable);
......@@ -89,15 +90,14 @@ v8::EmbedderGraph::Node* V8EmbedderGraphBuilder::GraphNode(
v8::EmbedderGraph::Node* V8EmbedderGraphBuilder::GraphNode(
Traceable traceable,
const char* name,
v8::EmbedderGraph::Node* wrapper) const {
const char* name) const {
auto iter = graph_node_.find(traceable);
if (iter != graph_node_.end())
return iter->value;
// Ownership of the new node is transferred to the graph_.
// graph_node_.at(tracable) is valid for all BuildEmbedderGraph execution.
auto node = graph_->AddNode(
std::unique_ptr<Graph::Node>(new EmbedderNode(name, wrapper)));
auto node =
graph_->AddNode(std::unique_ptr<Graph::Node>(new EmbedderNode(name)));
graph_node_.insert(traceable, node);
return node;
}
......
......@@ -36,22 +36,19 @@ class V8EmbedderGraphBuilder : public ScriptWrappableVisitor,
private:
class EmbedderNode : public Graph::Node {
public:
EmbedderNode(const char* name, Graph::Node* wrapper)
: name_(name), wrapper_(wrapper) {}
explicit EmbedderNode(const char* name) : name_(name) {}
// Graph::Node overrides.
const char* Name() override { return name_; }
size_t SizeInBytes() override { return 0; }
Graph::Node* WrapperNode() override { return wrapper_; }
private:
const char* name_;
Graph::Node* wrapper_;
};
class EmbedderRootNode : public EmbedderNode {
public:
explicit EmbedderRootNode(const char* name) : EmbedderNode(name, nullptr) {}
explicit EmbedderRootNode(const char* name) : EmbedderNode(name) {}
// Graph::Node override.
bool IsRootNode() { return true; }
};
......@@ -81,9 +78,7 @@ class V8EmbedderGraphBuilder : public ScriptWrappableVisitor,
const TraceWrapperDescriptor&) const;
Graph::Node* GraphNode(const v8::Local<v8::Value>&) const;
Graph::Node* GraphNode(Traceable,
const char* name,
Graph::Node* wrapper) const;
Graph::Node* GraphNode(Traceable, const char* name) const;
void VisitPendingActivities();
void VisitTransitiveClosure();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment