From: mikhail.naganov@gmail.com Date: Fri, 1 Oct 2010 07:19:23 +0000 (+0000) Subject: Fix HeapSnapshotsDiff test, diff implementation, and a bug introduced X-Git-Tag: upstream/4.7.83~21129 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=5a30072502d8a1d7a44d8d8c7e8d2bfdf5e986c1;p=platform%2Fupstream%2Fv8.git 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 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@5570 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- diff --git a/src/profile-generator.cc b/src/profile-generator.cc index 525dea2fb..08ee04697 100644 --- a/src/profile-generator.cc +++ b/src/profile-generator.cc @@ -952,7 +952,7 @@ void HeapEntry::PaintAllReachable() { 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 @@ HeapSnapshot::HeapSnapshot(HeapSnapshotsCollection* collection, 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 @@ HeapEntry* HeapSnapshot::AddEntry(HeapObject* object, 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 @@ HeapSnapshotsComparator::~HeapSnapshotsComparator() { HeapSnapshotsDiff* HeapSnapshotsComparator::Compare(HeapSnapshot* snapshot1, HeapSnapshot* snapshot2) { + snapshot1->ClearPaint(); + snapshot1->root()->PaintAllReachable(); + snapshot2->ClearPaint(); + snapshot2->root()->PaintAllReachable(); + List* entries1 = snapshot1->GetSortedEntriesList(); List* entries2 = snapshot2->GetSortedEntriesList(); int i = 0, j = 0; @@ -2103,8 +2108,14 @@ HeapSnapshotsDiff* HeapSnapshotsComparator::Compare(HeapSnapshot* snapshot1, 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); @@ -2122,35 +2133,17 @@ HeapSnapshotsDiff* HeapSnapshotsComparator::Compare(HeapSnapshot* snapshot1, 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); + diff->CreateRoots(added_entries.length(), deleted_entries.length()); - int del_child_index = 0, deleted_entry_index = 1; 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); + diff->AddDeletedEntry(i, i + 1, entry); } - int add_child_index = 0, added_entry_index = 1; 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; } diff --git a/src/profile-generator.h b/src/profile-generator.h index 1e949a2cf..4206d29f0 100644 --- a/src/profile-generator.h +++ b/src/profile-generator.h @@ -662,7 +662,7 @@ class HeapSnapshot { 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 @@ class HeapSnapshot { Type type_; const char* title_; unsigned uid_; - int root_entry_index_; + HeapEntry* root_entry_; char* raw_entries_; List entries_; bool entries_sorted_; diff --git a/test/cctest/cctest.status b/test/cctest/cctest.status index d03f5f7a0..895e24539 100644 --- a/test/cctest/cctest.status +++ b/test/cctest/cctest.status @@ -35,11 +35,6 @@ test-debug/DebuggerAgent: PASS, (PASS || FAIL) if $system == linux # 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 diff --git a/test/cctest/test-heap-profiler.cc b/test/cctest/test-heap-profiler.cc index 5e570f34d..6340da53d 100644 --- a/test/cctest/test-heap-profiler.cc +++ b/test/cctest/test-heap-profiler.cc @@ -787,6 +787,7 @@ TEST(HeapSnapshotsDiff) { 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 @@ TEST(HeapSnapshotsDiff) { 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 @@ TEST(HeapSnapshotsDiff) { 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; @@ -849,6 +850,19 @@ TEST(HeapSnapshotsDiff) { } +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(reinterpret_cast( + snapshot))->GetSortedEntriesList(); + const v8::HeapGraphNode* root2 = snapshot->GetRoot(); + CHECK_EQ(root1, root2); +} + + namespace v8 { namespace internal {