From: yurys@chromium.org Date: Fri, 6 Apr 2012 14:16:45 +0000 (+0000) Subject: We can avoid putting all nodes into a hash map from HeapEntry to ID and sorting that... X-Git-Tag: upstream/4.7.83~16946 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=2f0e0afb091544c066b2c0d80276d7989ff7a3c9;p=platform%2Fupstream%2Fv8.git We can avoid putting all nodes into a hash map from HeapEntry to ID and sorting that map as the nodes are already stored in right order in HeapSnapshot::entries_ list. Review URL: https://chromiumcodereview.appspot.com/10012013 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@11245 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- diff --git a/src/profile-generator.cc b/src/profile-generator.cc index c81354f..5138d9d 100644 --- a/src/profile-generator.cc +++ b/src/profile-generator.cc @@ -975,6 +975,7 @@ void HeapEntry::Init(HeapSnapshot* snapshot, name_ = name; self_size_ = self_size; retained_size_ = 0; + entry_index_ = -1; children_count_ = children_count; retainers_count_ = retainers_count; dominator_ = NULL; @@ -1108,7 +1109,7 @@ template struct SnapshotSizeConstants; template <> struct SnapshotSizeConstants<4> { static const int kExpectedHeapGraphEdgeSize = 12; - static const int kExpectedHeapEntrySize = 32; + static const int kExpectedHeapEntrySize = 36; static const size_t kMaxSerializableSnapshotRawSize = 256 * MB; }; @@ -1133,7 +1134,6 @@ HeapSnapshot::HeapSnapshot(HeapSnapshotsCollection* collection, gc_roots_entry_(NULL), natives_root_entry_(NULL), raw_entries_(NULL), - entries_sorted_(false), max_snapshot_js_object_id_(0) { STATIC_CHECK( sizeof(HeapGraphEdge) == @@ -1185,6 +1185,7 @@ void HeapSnapshot::ClearPaint() { HeapEntry* HeapSnapshot::AddRootEntry(int children_count) { ASSERT(root_entry_ == NULL); + ASSERT(entries_.is_empty()); // Root entry must be the first one. return (root_entry_ = AddEntry(HeapEntry::kObject, "", HeapObjectsMap::kInternalRootObjectId, @@ -1285,11 +1286,11 @@ static int SortByIds(const T* entry1_ptr, List* HeapSnapshot::GetSortedEntriesList() { - if (!entries_sorted_) { - entries_.Sort(SortByIds); - entries_sorted_ = true; + if (sorted_entries_.is_empty()) { + sorted_entries_.AddAll(entries_); + sorted_entries_.Sort(SortByIds); } - return &entries_; + return &sorted_entries_; } @@ -1514,20 +1515,39 @@ HeapEntriesMap::~HeapEntriesMap() { } -void HeapEntriesMap::AllocateEntries() { - for (HashMap::Entry* p = entries_.Start(); - p != NULL; - p = entries_.Next(p)) { - EntryInfo* entry_info = reinterpret_cast(p->value); +void HeapEntriesMap::AllocateHeapEntryForMapEntry(HashMap::Entry* map_entry) { + EntryInfo* entry_info = reinterpret_cast(map_entry->value); entry_info->entry = entry_info->allocator->AllocateEntry( - p->key, + map_entry->key, entry_info->children_count, entry_info->retainers_count); ASSERT(entry_info->entry != NULL); ASSERT(entry_info->entry != kHeapEntryPlaceholder); entry_info->children_count = 0; entry_info->retainers_count = 0; +} + + +void HeapEntriesMap::AllocateEntries(HeapThing root_object) { + HashMap::Entry* root_entry = + entries_.Lookup(root_object, Hash(root_object), false); + ASSERT(root_entry != NULL); + // Make sure root entry is allocated first. + AllocateHeapEntryForMapEntry(root_entry); + void* root_entry_value = root_entry->value; + // Remove the root object from map while iterating through other entries. + entries_.Remove(root_object, Hash(root_object)); + root_entry = NULL; + + for (HashMap::Entry* p = entries_.Start(); + p != NULL; + p = entries_.Next(p)) { + AllocateHeapEntryForMapEntry(p); } + + // Insert root entry back. + root_entry = entries_.Lookup(root_object, Hash(root_object), true); + root_entry->value = root_entry_value; } @@ -3106,7 +3126,7 @@ bool HeapSnapshotGenerator::GenerateSnapshot() { entries_.total_retainers_count()); // Allocate heap objects to entries hash map. - entries_.AllocateEntries(); + entries_.AllocateEntries(V8HeapExplorer::kInternalRootObject); // Pass 2. Fill references. if (!FillReferences()) return false; @@ -3417,7 +3437,6 @@ void HeapSnapshotJSONSerializer::Serialize(v8::OutputStream* stream) { } // Since nodes graph is cyclic, we need the first pass to enumerate // them. Strings can be serialized in one pass. - EnumerateNodes(); SerializeImpl(); delete writer_; @@ -3470,34 +3489,6 @@ void HeapSnapshotJSONSerializer::SerializeImpl() { } -class HeapSnapshotJSONSerializerEnumerator { - public: - explicit HeapSnapshotJSONSerializerEnumerator(HeapSnapshotJSONSerializer* s) - : s_(s) { - } - void Apply(HeapEntry** entry) { - s_->GetNodeId(*entry); - } - private: - HeapSnapshotJSONSerializer* s_; -}; - -void HeapSnapshotJSONSerializer::EnumerateNodes() { - GetNodeId(snapshot_->root()); // Make sure root gets the first id. - HeapSnapshotJSONSerializerEnumerator iter(this); - snapshot_->IterateEntries(&iter); -} - - -int HeapSnapshotJSONSerializer::GetNodeId(HeapEntry* entry) { - HashMap::Entry* cache_entry = nodes_.Lookup(entry, ObjectHash(entry), true); - if (cache_entry->value == NULL) { - cache_entry->value = reinterpret_cast(next_node_id_++); - } - return static_cast(reinterpret_cast(cache_entry->value)); -} - - int HeapSnapshotJSONSerializer::GetStringId(const char* s) { HashMap::Entry* cache_entry = strings_.Lookup( const_cast(s), ObjectHash(s), true); @@ -3548,7 +3539,7 @@ void HeapSnapshotJSONSerializer::SerializeEdge(HeapGraphEdge* edge) { buffer[buffer_pos++] = ','; buffer_pos = itoa(edge_name_or_index, buffer, buffer_pos); buffer[buffer_pos++] = ','; - buffer_pos = itoa(GetNodeId(edge->to()), buffer, buffer_pos); + buffer_pos = itoa(edge->to()->entry_index(), buffer, buffer_pos); buffer[buffer_pos++] = '\0'; writer_->AddString(buffer.start()); } @@ -3575,7 +3566,7 @@ void HeapSnapshotJSONSerializer::SerializeNode(HeapEntry* entry) { buffer[buffer_pos++] = ','; buffer_pos = itoa(entry->retained_size(), buffer, buffer_pos); buffer[buffer_pos++] = ','; - buffer_pos = itoa(GetNodeId(entry->dominator()), buffer, buffer_pos); + buffer_pos = itoa(entry->dominator()->entry_index(), buffer, buffer_pos); buffer[buffer_pos++] = ','; buffer_pos = itoa(children.length(), buffer, buffer_pos); buffer[buffer_pos++] = '\0'; @@ -3645,23 +3636,25 @@ void HeapSnapshotJSONSerializer::SerializeNodes() { const int node_fields_count = 7; // type,name,id,self_size,retained_size,dominator,children_count. const int edge_fields_count = 3; // type,name|index,to_node. - List sorted_nodes; - SortHashMap(&nodes_, &sorted_nodes); - // Rewrite node ids, so they refer to actual array positions. - if (sorted_nodes.length() > 1) { + + List& nodes = *(snapshot_->entries()); + // Root must be the first. + ASSERT(nodes.first() == snapshot_->root()); + // Rewrite node indexes, so they refer to actual array positions. Do this + // only once. + if (nodes[0]->entry_index() == -1) { // Nodes start from array index 1. - int prev_value = 1; - sorted_nodes[0]->value = reinterpret_cast(prev_value); - for (int i = 1; i < sorted_nodes.length(); ++i) { - HeapEntry* prev_heap_entry = - reinterpret_cast(sorted_nodes[i-1]->key); - prev_value += node_fields_count + - prev_heap_entry->children().length() * edge_fields_count; - sorted_nodes[i]->value = reinterpret_cast(prev_value); + int index = 1; + for (int i = 0; i < nodes.length(); ++i) { + HeapEntry* node = nodes[i]; + node->set_entry_index(index); + index += node_fields_count + + node->children().length() * edge_fields_count; } } - for (int i = 0; i < sorted_nodes.length(); ++i) { - SerializeNode(reinterpret_cast(sorted_nodes[i]->key)); + + for (int i = 0; i < nodes.length(); ++i) { + SerializeNode(nodes[i]); if (writer_->aborted()) return; } } diff --git a/src/profile-generator.h b/src/profile-generator.h index b9de69b..1fa647e 100644 --- a/src/profile-generator.h +++ b/src/profile-generator.h @@ -549,6 +549,8 @@ class HeapEntry BASE_EMBEDDED { void set_retained_size(int value) { retained_size_ = value; } int ordered_index() { return ordered_index_; } void set_ordered_index(int value) { ordered_index_ = value; } + int entry_index() { return entry_index_; } + void set_entry_index(int value) { entry_index_ = value; } Vector children() { return Vector(children_arr(), children_count_); } @@ -606,6 +608,7 @@ class HeapEntry BASE_EMBEDDED { int ordered_index_; // Used during dominator tree building. int retained_size_; // At that moment, there is no retained size yet. }; + int entry_index_; SnapshotObjectId id_; HeapEntry* dominator_; HeapSnapshot* snapshot_; @@ -667,8 +670,6 @@ class HeapSnapshot { void ClearPaint(); HeapEntry* GetEntryById(SnapshotObjectId id); List* GetSortedEntriesList(); - template - void IterateEntries(Visitor* visitor) { entries_.Iterate(visitor); } void SetDominatorsToSelf(); void Print(int max_depth); @@ -687,7 +688,7 @@ class HeapSnapshot { HeapEntry* gc_subroot_entries_[VisitorSynchronization::kNumberOfSyncTags]; char* raw_entries_; List entries_; - bool entries_sorted_; + List sorted_entries_; size_t raw_entries_size_; SnapshotObjectId max_snapshot_js_object_id_; @@ -815,7 +816,7 @@ class HeapEntriesMap { HeapEntriesMap(); ~HeapEntriesMap(); - void AllocateEntries(); + void AllocateEntries(HeapThing root_object); HeapEntry* Map(HeapThing thing); void Pair(HeapThing thing, HeapEntriesAllocator* allocator, HeapEntry* entry); void CountReference(HeapThing from, HeapThing to, @@ -842,6 +843,8 @@ class HeapEntriesMap { int retainers_count; }; + static inline void AllocateHeapEntryForMapEntry(HashMap::Entry* map_entry); + static uint32_t Hash(HeapThing thing) { return ComputeIntegerHash( static_cast(reinterpret_cast(thing)), @@ -1122,7 +1125,6 @@ class HeapSnapshotJSONSerializer { public: explicit HeapSnapshotJSONSerializer(HeapSnapshot* snapshot) : snapshot_(snapshot), - nodes_(ObjectsMatch), strings_(ObjectsMatch), next_node_id_(1), next_string_id_(1), @@ -1141,9 +1143,7 @@ class HeapSnapshotJSONSerializer { v8::internal::kZeroHashSeed); } - void EnumerateNodes(); HeapSnapshot* CreateFakeSnapshot(); - int GetNodeId(HeapEntry* entry); int GetStringId(const char* s); void SerializeEdge(HeapGraphEdge* edge); void SerializeImpl(); @@ -1157,7 +1157,6 @@ class HeapSnapshotJSONSerializer { static const int kMaxSerializableSnapshotRawSize; HeapSnapshot* snapshot_; - HashMap nodes_; HashMap strings_; int next_node_id_; int next_string_id_;