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
This commit is contained in:
mikhail.naganov@gmail.com 2010-10-01 07:19:23 +00:00
parent 7228d867cb
commit 5a30072502
4 changed files with 39 additions and 37 deletions

View File

@ -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<HeapEntry*>* entries1 = snapshot1->GetSortedEntriesList();
List<HeapEntry*>* 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;
}

View File

@ -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<HeapEntry*> entries_;
bool entries_sorted_;

View File

@ -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

View File

@ -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<i::HeapSnapshot*>(reinterpret_cast<const i::HeapSnapshot*>(
snapshot))->GetSortedEntriesList();
const v8::HeapGraphNode* root2 = snapshot->GetRoot();
CHECK_EQ(root1, root2);
}
namespace v8 {
namespace internal {