Revision: 5570
Author: [email protected]
Date: Fri Oct 1 00:19:23 2010
Log: Fix HeapSnapshotsDiff test, diff implementation, and a bug introduced
during snapshot size optimization.
Sorry, now I figured out that the diff implementation itself was also
incorrect. Reachable nodes must be filtered from the beginning,
otherwise, an object that is already disconnected, but not discarded
yet, will not appear as a deleted (thankfully, this bug for some
reason had appeared on the x64 port.)
BUG=868
TEST=HeapSnapshotRootPreservedAfterSorting
Review URL: http://codereview.chromium.org/3531005
http://code.google.com/p/v8/source/detail?r=5570
Modified:
/branches/bleeding_edge/src/profile-generator.cc
/branches/bleeding_edge/src/profile-generator.h
/branches/bleeding_edge/test/cctest/cctest.status
/branches/bleeding_edge/test/cctest/test-heap-profiler.cc
=======================================
--- /branches/bleeding_edge/src/profile-generator.cc Thu Sep 30 05:35:03
2010
+++ /branches/bleeding_edge/src/profile-generator.cc Fri Oct 1 00:19:23
2010
@@ -952,7 +952,7 @@
void HeapEntry::Print(int max_depth, int indent) {
- OS::Print("%6d %6d %6d [%ld] ",
+ OS::Print("%6d %6d %6d [%llu] ",
self_size(), ReachableSize(), RetainedSize(), id_);
if (type() != kString) {
OS::Print("%s %.40s\n", TypeAsString(), name_);
@@ -1237,7 +1237,7 @@
type_(type),
title_(title),
uid_(uid),
- root_entry_index_(-1),
+ root_entry_(NULL),
raw_entries_(NULL),
entries_sorted_(false) {
STATIC_ASSERT(
@@ -1276,11 +1276,11 @@
int children_count,
int retainers_count) {
if (object == kInternalRootObject) {
- ASSERT(root_entry_index_ == -1);
- root_entry_index_ = entries_.length();
+ ASSERT(root_entry_ == NULL);
ASSERT(retainers_count == 0);
- return AddEntry(
+ root_entry_ = AddEntry(
HeapEntry::kInternal, "", 0, 0, children_count, retainers_count);
+ return root_entry_;
} else if (object->IsJSFunction()) {
JSFunction* func = JSFunction::cast(object);
SharedFunctionInfo* shared = func->shared();
@@ -2095,6 +2095,11 @@
HeapSnapshotsDiff* HeapSnapshotsComparator::Compare(HeapSnapshot*
snapshot1,
HeapSnapshot*
snapshot2) {
+ snapshot1->ClearPaint();
+ snapshot1->root()->PaintAllReachable();
+ snapshot2->ClearPaint();
+ snapshot2->root()->PaintAllReachable();
+
List<HeapEntry*>* entries1 = snapshot1->GetSortedEntriesList();
List<HeapEntry*>* entries2 = snapshot2->GetSortedEntriesList();
int i = 0, j = 0;
@@ -2103,8 +2108,14 @@
uint64_t id1 = entries1->at(i)->id();
uint64_t id2 = entries2->at(j)->id();
if (id1 == id2) {
- i++;
- j++;
+ HeapEntry* entry1 = entries1->at(i++);
+ HeapEntry* entry2 = entries2->at(j++);
+ if (entry1->painted_reachable() != entry2->painted_reachable()) {
+ if (entry1->painted_reachable())
+ deleted_entries.Add(entry1);
+ else
+ added_entries.Add(entry2);
+ }
} else if (id1 < id2) {
HeapEntry* entry = entries1->at(i++);
deleted_entries.Add(entry);
@@ -2121,36 +2132,18 @@
HeapEntry* entry = entries2->at(j++);
added_entries.Add(entry);
}
-
- snapshot1->ClearPaint();
- snapshot1->root()->PaintAllReachable();
- snapshot2->ClearPaint();
- snapshot2->root()->PaintAllReachable();
- int reachable_deleted_entries = 0, reachable_added_entries = 0;
- for (int i = 0; i < deleted_entries.length(); ++i) {
- HeapEntry* entry = deleted_entries[i];
- if (entry->painted_reachable()) ++reachable_deleted_entries;
- }
- for (int i = 0; i < added_entries.length(); ++i) {
- HeapEntry* entry = added_entries[i];
- if (entry->painted_reachable()) ++reachable_added_entries;
- }
HeapSnapshotsDiff* diff = new HeapSnapshotsDiff(snapshot1, snapshot2);
diffs_.Add(diff);
- diff->CreateRoots(reachable_added_entries, reachable_deleted_entries);
-
- int del_child_index = 0, deleted_entry_index = 1;
+ diff->CreateRoots(added_entries.length(), deleted_entries.length());
+
for (int i = 0; i < deleted_entries.length(); ++i) {
HeapEntry* entry = deleted_entries[i];
- if (entry->painted_reachable())
- diff->AddDeletedEntry(del_child_index++, deleted_entry_index++,
entry);
- }
- int add_child_index = 0, added_entry_index = 1;
+ diff->AddDeletedEntry(i, i + 1, entry);
+ }
for (int i = 0; i < added_entries.length(); ++i) {
HeapEntry* entry = added_entries[i];
- if (entry->painted_reachable())
- diff->AddAddedEntry(add_child_index++, added_entry_index++, entry);
+ diff->AddAddedEntry(i, i + 1, entry);
}
return diff;
}
=======================================
--- /branches/bleeding_edge/src/profile-generator.h Thu Sep 30 05:35:03 2010
+++ /branches/bleeding_edge/src/profile-generator.h Fri Oct 1 00:19:23 2010
@@ -662,7 +662,7 @@
Type type() { return type_; }
const char* title() { return title_; }
unsigned uid() { return uid_; }
- HeapEntry* root() { return entries_[root_entry_index_]; }
+ HeapEntry* root() { return root_entry_; }
void AllocateEntries(
int entries_count, int children_count, int retainers_count);
@@ -704,7 +704,7 @@
Type type_;
const char* title_;
unsigned uid_;
- int root_entry_index_;
+ HeapEntry* root_entry_;
char* raw_entries_;
List<HeapEntry*> entries_;
bool entries_sorted_;
=======================================
--- /branches/bleeding_edge/test/cctest/cctest.status Thu Sep 30 05:35:03
2010
+++ /branches/bleeding_edge/test/cctest/cctest.status Fri Oct 1 00:19:23
2010
@@ -35,11 +35,6 @@
# BUG(382): Weird test. Can't guarantee that it never times out.
test-api/ApplyInterruption: PASS || TIMEOUT
-# Bug (484): This test which we thought was originally corrected in r5236
-# is reappering. Disabled until bug in test is fixed. This only fails
-# when snapshot is on, so I am marking it PASS || FAIL
-test-heap-profiler/HeapSnapshotsDiff: PASS || FAIL
-
# These tests always fail. They are here to test test.py. If
# they don't fail then test.py has failed.
test-serialize/TestThatAlwaysFails: FAIL
=======================================
--- /branches/bleeding_edge/test/cctest/test-heap-profiler.cc Thu Sep 30
05:35:03 2010
+++ /branches/bleeding_edge/test/cctest/test-heap-profiler.cc Fri Oct 1
00:19:23 2010
@@ -787,6 +787,7 @@
CompileAndRunScript(
"function A() {}\n"
"function B(x) { this.x = x; }\n"
+ "function A2(a) { for (var i = 0; i < a; ++i) this[i] = i; }\n"
"var a = new A();\n"
"var b = new B(a);");
const v8::HeapSnapshot* snapshot1 =
@@ -795,7 +796,7 @@
CompileAndRunScript(
"delete a;\n"
"b.x = null;\n"
- "var a = new A();\n"
+ "var a = new A2(20);\n"
"var b2 = new B(a);");
const v8::HeapSnapshot* snapshot2 =
v8::HeapProfiler::TakeSnapshot(v8::String::New("s2"));
@@ -811,7 +812,7 @@
const v8::HeapGraphNode* node = prop->GetToNode();
if (node->GetType() == v8::HeapGraphNode::kObject) {
v8::String::AsciiValue node_name(node->GetName());
- if (strcmp(*node_name, "A") == 0) {
+ if (strcmp(*node_name, "A2") == 0) {
CHECK(IsNodeRetainedAs(node, v8::HeapGraphEdge::kProperty, "a"));
CHECK(!found_A);
found_A = true;
@@ -847,6 +848,19 @@
CHECK_NE_UINT64_T(0, s1_A_id);
CHECK(s1_A_id != s2_A_id);
}
+
+
+TEST(HeapSnapshotRootPreservedAfterSorting) {
+ v8::HandleScope scope;
+ LocalContext env;
+ const v8::HeapSnapshot* snapshot =
+ v8::HeapProfiler::TakeSnapshot(v8::String::New("s"));
+ const v8::HeapGraphNode* root1 = snapshot->GetRoot();
+ const_cast<i::HeapSnapshot*>(reinterpret_cast<const i::HeapSnapshot*>(
+ snapshot))->GetSortedEntriesList();
+ const v8::HeapGraphNode* root2 = snapshot->GetRoot();
+ CHECK_EQ(root1, root2);
+}
namespace v8 {
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
