[cpu-profiler] Eagerly delete not used CodeEntry'es

Currently ProfilerListener holds all the CodeEntries it ever
created during the profiling session. It is not capable of removing
entries corresponding to the code objects discarded by GC as there's
no such code event.

However it is sometimes possible to tell if a code object was GCed.
Hook up to the CodeMap code entry removal and if the entry has never
been hit by a sample we can safely delete it.

As a bonus the CodeEntryInfo size has been reduced on x64, which also
saves 8 x <number of code entries> bytes.

BUG=v8:7719

Change-Id: I988bc5b59f3fba07157a9f472cbcf68596fcd969
Reviewed-on: https://chromium-review.googlesource.com/1054346Reviewed-by: Peter Marshall <petermarshall@chromium.org>
Commit-Queue: Alexei Filippov <alph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#53222}

src/profiler/cpu-profiler.cc

@@ -319,19 +319,18 @@ void CpuProfiler::ResetProfiles() {
   profiles_.reset(new CpuProfilesCollection(isolate_));
   profiles_->set_cpu_profiler(this);
   profiler_listener_.reset();
+  generator_.reset();
 }
 
 void CpuProfiler::CreateEntriesForRuntimeCallStats() {
-  static_entries_.clear();
   RuntimeCallStats* rcs = isolate_->counters()->runtime_call_stats();
   CodeMap* code_map = generator_->code_map();
   for (int i = 0; i < RuntimeCallStats::kNumberOfCounters; ++i) {
     RuntimeCallCounter* counter = rcs->GetCounter(i);
     DCHECK(counter->name());
-    std::unique_ptr<CodeEntry> entry(new CodeEntry(
-        CodeEventListener::FUNCTION_TAG, counter->name(), "native V8Runtime"));
-    code_map->AddCode(reinterpret_cast<Address>(counter), entry.get(), 1);
-    static_entries_.push_back(std::move(entry));
+    auto entry = new CodeEntry(CodeEventListener::FUNCTION_TAG, counter->name(),
+                               "native V8Runtime");
+    code_map->AddCode(reinterpret_cast<Address>(counter), entry, 1);
   }
 }
 
@@ -369,13 +368,15 @@ void CpuProfiler::StartProcessorIfNotStarted() {
   // Disable logging when using the new implementation.
   saved_is_logging_ = logger->is_logging_;
   logger->is_logging_ = false;
-  generator_.reset(new ProfileGenerator(profiles_.get()));
+  if (!generator_) {
+    generator_.reset(new ProfileGenerator(profiles_.get()));
+    CreateEntriesForRuntimeCallStats();
+  }
   processor_.reset(new ProfilerEventsProcessor(isolate_, generator_.get(),
                                                sampling_interval_));
   if (!profiler_listener_) {
     profiler_listener_.reset(new ProfilerListener(isolate_, this));
   }
-  CreateEntriesForRuntimeCallStats();
   logger->AddCodeEventListener(profiler_listener_.get());
   is_profiling_ = true;
   isolate_->set_is_profiling(true);
@@ -414,7 +415,6 @@ void CpuProfiler::StopProcessor() {
   logger->RemoveCodeEventListener(profiler_listener_.get());
   processor_->StopSynchronously();
   processor_.reset();
-  generator_.reset();
   logger->is_logging_ = saved_is_logging_;
 }
 

src/profiler/cpu-profiler.h

@@ -234,7 +234,6 @@ class CpuProfiler : public CodeEventObserver {
   std::unique_ptr<ProfileGenerator> generator_;
   std::unique_ptr<ProfilerEventsProcessor> processor_;
   std::unique_ptr<ProfilerListener> profiler_listener_;
-  std::vector<std::unique_ptr<CodeEntry>> static_entries_;
   bool saved_is_logging_;
   bool is_profiling_;
 
@@ -244,5 +243,4 @@ class CpuProfiler : public CodeEventObserver {
 }  // namespace internal
 }  // namespace v8
 
-
 #endif  // V8_PROFILER_CPU_PROFILER_H_

src/profiler/profile-generator-inl.h

@@ -27,6 +27,12 @@ CodeEntry::CodeEntry(CodeEventListener::LogEventsAndTags tag, const char* name,
       line_info_(std::move(line_info)),
       instruction_start_(instruction_start) {}
 
+inline CodeEntry* ProfileGenerator::FindEntry(Address address) {
+  CodeEntry* entry = code_map_.FindEntry(address);
+  if (entry) entry->mark_used();
+  return entry;
+}
+
 ProfileNode::ProfileNode(ProfileTree* tree, CodeEntry* entry,
                          ProfileNode* parent)
     : tree_(tree),

src/profiler/profile-generator.cc

@@ -478,20 +478,28 @@ void CpuProfile::Print() {
 }
 
 CodeMap::CodeMap() = default;
+CodeMap::~CodeMap() = default;
 
 void CodeMap::AddCode(Address addr, CodeEntry* entry, unsigned size) {
-  DeleteAllCoveredCode(addr, addr + size);
-  code_map_.insert({addr, CodeEntryInfo(entry, size)});
+  ClearCodesInRange(addr, addr + size);
+  code_map_.emplace(
+      addr, CodeEntryInfo{static_cast<unsigned>(code_entries_.size()), size});
+  code_entries_.push_back(std::unique_ptr<CodeEntry>(entry));
 }
 
-void CodeMap::DeleteAllCoveredCode(Address start, Address end) {
+void CodeMap::ClearCodesInRange(Address start, Address end) {
   auto left = code_map_.upper_bound(start);
   if (left != code_map_.begin()) {
     --left;
     if (left->first + left->second.size <= start) ++left;
   }
   auto right = left;
-  while (right != code_map_.end() && right->first < end) ++right;
+  for (; right != code_map_.end() && right->first < end; ++right) {
+    std::unique_ptr<CodeEntry>& entry = code_entries_[right->second.index];
+    if (!entry->used()) {
+      entry.reset();
+    }
+  }
   code_map_.erase(left, right);
 }
 
@@ -500,7 +508,10 @@ CodeEntry* CodeMap::FindEntry(Address addr) {
   if (it == code_map_.begin()) return nullptr;
   --it;
   Address end_address = it->first + it->second.size;
-  return addr < end_address ? it->second.entry : nullptr;
+  if (addr >= end_address) return nullptr;
+  CodeEntry* entry = code_entries_[it->second.index].get();
+  DCHECK(entry);
+  return entry;
 }
 
 void CodeMap::MoveCode(Address from, Address to) {
@@ -509,13 +520,15 @@ void CodeMap::MoveCode(Address from, Address to) {
   if (it == code_map_.end()) return;
   CodeEntryInfo info = it->second;
   code_map_.erase(it);
-  AddCode(to, info.entry, info.size);
+  DCHECK(from + info.size <= to || to + info.size <= from);
+  ClearCodesInRange(to, to + info.size);
+  code_map_.emplace(to, info);
 }
 
 void CodeMap::Print() {
   for (const auto& pair : code_map_) {
     base::OS::Print("%p %5d %s\n", reinterpret_cast<void*>(pair.first),
-                    pair.second.size, pair.second.entry->name());
+                    pair.second.size, code_entries_[pair.second.index]->name());
   }
 }
 

src/profiler/profile-generator.h

@@ -5,7 +5,9 @@
 #ifndef V8_PROFILER_PROFILE_GENERATOR_H_
 #define V8_PROFILER_PROFILE_GENERATOR_H_
 
+#include <deque>
 #include <map>
+#include <memory>
 #include <unordered_map>
 #include <vector>
 
@@ -74,6 +76,8 @@ class CodeEntry {
     rare_data_->deopt_reason_ = kNoDeoptReason;
     rare_data_->deopt_id_ = kNoDeoptimizationId;
   }
+  void mark_used() { bit_field_ = UsedField::update(bit_field_, true); }
+  bool used() const { return UsedField::decode(bit_field_); }
 
   void FillFunctionInfo(SharedFunctionInfo* shared);
 
@@ -154,8 +158,9 @@ class CodeEntry {
   static base::LazyDynamicInstance<CodeEntry, UnresolvedEntryCreateTrait>::type
       kUnresolvedEntry;
 
-  class TagField : public BitField<Logger::LogEventsAndTags, 0, 8> {};
-  class BuiltinIdField : public BitField<Builtins::Name, 8, 24> {};
+  using TagField = BitField<Logger::LogEventsAndTags, 0, 8>;
+  using BuiltinIdField = BitField<Builtins::Name, 8, 23>;
+  using UsedField = BitField<bool, 31, 1>;
 
   uint32_t bit_field_;
   const char* name_;
@@ -319,6 +324,7 @@ class CpuProfile {
 class CodeMap {
  public:
   CodeMap();
+  ~CodeMap();
 
   void AddCode(Address addr, CodeEntry* entry, unsigned size);
   void MoveCode(Address from, Address to);
@@ -327,14 +333,13 @@ class CodeMap {
 
  private:
   struct CodeEntryInfo {
-    CodeEntryInfo(CodeEntry* an_entry, unsigned a_size)
-        : entry(an_entry), size(a_size) {}
-    CodeEntry* entry;
+    unsigned index;
     unsigned size;
   };
 
-  void DeleteAllCoveredCode(Address start, Address end);
+  void ClearCodesInRange(Address start, Address end);
 
+  std::deque<std::unique_ptr<CodeEntry>> code_entries_;
   std::map<Address, CodeEntryInfo> code_map_;
 
   DISALLOW_COPY_AND_ASSIGN(CodeMap);
@@ -374,7 +379,6 @@ class CpuProfilesCollection {
   DISALLOW_COPY_AND_ASSIGN(CpuProfilesCollection);
 };
 
-
 class ProfileGenerator {
  public:
   explicit ProfileGenerator(CpuProfilesCollection* profiles);
@@ -384,7 +388,7 @@ class ProfileGenerator {
   CodeMap* code_map() { return &code_map_; }
 
  private:
-  CodeEntry* FindEntry(Address address) { return code_map_.FindEntry(address); }
+  CodeEntry* FindEntry(Address address);
   CodeEntry* EntryForVMState(StateTag tag);
 
   CpuProfilesCollection* profiles_;
@@ -393,7 +397,6 @@ class ProfileGenerator {
   DISALLOW_COPY_AND_ASSIGN(ProfileGenerator);
 };
 
-
 }  // namespace internal
 }  // namespace v8
 

src/profiler/profiler-listener.cc

@@ -301,12 +301,8 @@ CodeEntry* ProfilerListener::NewCodeEntry(
     CodeEventListener::LogEventsAndTags tag, const char* name,
     const char* resource_name, int line_number, int column_number,
     std::unique_ptr<SourcePositionTable> line_info, Address instruction_start) {
-  std::unique_ptr<CodeEntry> code_entry = base::make_unique<CodeEntry>(
-      tag, name, resource_name, line_number, column_number,
-      std::move(line_info), instruction_start);
-  CodeEntry* raw_code_entry = code_entry.get();
-  code_entries_.push_back(std::move(code_entry));
-  return raw_code_entry;
+  return new CodeEntry(tag, name, resource_name, line_number, column_number,
+                       std::move(line_info), instruction_start);
 }
 
 }  // namespace internal

src/profiler/profiler-listener.h

@@ -5,6 +5,7 @@
 #ifndef V8_PROFILER_PROFILER_LISTENER_H_
 #define V8_PROFILER_PROFILER_LISTENER_H_
 
+#include <memory>
 #include <vector>
 
 #include "src/code-events.h"
@@ -18,7 +19,7 @@ class CodeEventsContainer;
 class CodeEventObserver {
  public:
   virtual void CodeEventHandler(const CodeEventsContainer& evt_rec) = 0;
-  virtual ~CodeEventObserver() {}
+  virtual ~CodeEventObserver() = default;
 };
 
 class ProfilerListener : public CodeEventListener {
@@ -87,7 +88,6 @@ class ProfilerListener : public CodeEventListener {
   Isolate* isolate_;
   CodeEventObserver* observer_;
   StringsStorage function_and_resource_names_;
-  std::deque<std::unique_ptr<CodeEntry>> code_entries_;
 
   DISALLOW_COPY_AND_ASSIGN(ProfilerListener);
 };

test/cctest/test-profile-generator.cc

@@ -268,52 +268,50 @@ static inline void* ToPointer(int n) { return reinterpret_cast<void*>(n); }
 
 TEST(CodeMapAddCode) {
   CodeMap code_map;
-  CodeEntry entry1(i::CodeEventListener::FUNCTION_TAG, "aaa");
-  CodeEntry entry2(i::CodeEventListener::FUNCTION_TAG, "bbb");
-  CodeEntry entry3(i::CodeEventListener::FUNCTION_TAG, "ccc");
-  CodeEntry entry4(i::CodeEventListener::FUNCTION_TAG, "ddd");
-  code_map.AddCode(ToAddress(0x1500), &entry1, 0x200);
-  code_map.AddCode(ToAddress(0x1700), &entry2, 0x100);
-  code_map.AddCode(ToAddress(0x1900), &entry3, 0x50);
-  code_map.AddCode(ToAddress(0x1950), &entry4, 0x10);
+  CodeEntry* entry1 = new CodeEntry(i::CodeEventListener::FUNCTION_TAG, "aaa");
+  CodeEntry* entry2 = new CodeEntry(i::CodeEventListener::FUNCTION_TAG, "bbb");
+  CodeEntry* entry3 = new CodeEntry(i::CodeEventListener::FUNCTION_TAG, "ccc");
+  CodeEntry* entry4 = new CodeEntry(i::CodeEventListener::FUNCTION_TAG, "ddd");
+  code_map.AddCode(ToAddress(0x1500), entry1, 0x200);
+  code_map.AddCode(ToAddress(0x1700), entry2, 0x100);
+  code_map.AddCode(ToAddress(0x1900), entry3, 0x50);
+  code_map.AddCode(ToAddress(0x1950), entry4, 0x10);
   CHECK(!code_map.FindEntry(0));
   CHECK(!code_map.FindEntry(ToAddress(0x1500 - 1)));
-  CHECK_EQ(&entry1, code_map.FindEntry(ToAddress(0x1500)));
-  CHECK_EQ(&entry1, code_map.FindEntry(ToAddress(0x1500 + 0x100)));
-  CHECK_EQ(&entry1, code_map.FindEntry(ToAddress(0x1500 + 0x200 - 1)));
-  CHECK_EQ(&entry2, code_map.FindEntry(ToAddress(0x1700)));
-  CHECK_EQ(&entry2, code_map.FindEntry(ToAddress(0x1700 + 0x50)));
-  CHECK_EQ(&entry2, code_map.FindEntry(ToAddress(0x1700 + 0x100 - 1)));
+  CHECK_EQ(entry1, code_map.FindEntry(ToAddress(0x1500)));
+  CHECK_EQ(entry1, code_map.FindEntry(ToAddress(0x1500 + 0x100)));
+  CHECK_EQ(entry1, code_map.FindEntry(ToAddress(0x1500 + 0x200 - 1)));
+  CHECK_EQ(entry2, code_map.FindEntry(ToAddress(0x1700)));
+  CHECK_EQ(entry2, code_map.FindEntry(ToAddress(0x1700 + 0x50)));
+  CHECK_EQ(entry2, code_map.FindEntry(ToAddress(0x1700 + 0x100 - 1)));
   CHECK(!code_map.FindEntry(ToAddress(0x1700 + 0x100)));
   CHECK(!code_map.FindEntry(ToAddress(0x1900 - 1)));
-  CHECK_EQ(&entry3, code_map.FindEntry(ToAddress(0x1900)));
-  CHECK_EQ(&entry3, code_map.FindEntry(ToAddress(0x1900 + 0x28)));
-  CHECK_EQ(&entry4, code_map.FindEntry(ToAddress(0x1950)));
-  CHECK_EQ(&entry4, code_map.FindEntry(ToAddress(0x1950 + 0x7)));
-  CHECK_EQ(&entry4, code_map.FindEntry(ToAddress(0x1950 + 0x10 - 1)));
+  CHECK_EQ(entry3, code_map.FindEntry(ToAddress(0x1900)));
+  CHECK_EQ(entry3, code_map.FindEntry(ToAddress(0x1900 + 0x28)));
+  CHECK_EQ(entry4, code_map.FindEntry(ToAddress(0x1950)));
+  CHECK_EQ(entry4, code_map.FindEntry(ToAddress(0x1950 + 0x7)));
+  CHECK_EQ(entry4, code_map.FindEntry(ToAddress(0x1950 + 0x10 - 1)));
   CHECK(!code_map.FindEntry(ToAddress(0x1950 + 0x10)));
   CHECK(!code_map.FindEntry(ToAddress(0xFFFFFFFF)));
 }
 
-
 TEST(CodeMapMoveAndDeleteCode) {
   CodeMap code_map;
-  CodeEntry entry1(i::CodeEventListener::FUNCTION_TAG, "aaa");
-  CodeEntry entry2(i::CodeEventListener::FUNCTION_TAG, "bbb");
-  code_map.AddCode(ToAddress(0x1500), &entry1, 0x200);
-  code_map.AddCode(ToAddress(0x1700), &entry2, 0x100);
-  CHECK_EQ(&entry1, code_map.FindEntry(ToAddress(0x1500)));
-  CHECK_EQ(&entry2, code_map.FindEntry(ToAddress(0x1700)));
+  CodeEntry* entry1 = new CodeEntry(i::CodeEventListener::FUNCTION_TAG, "aaa");
+  CodeEntry* entry2 = new CodeEntry(i::CodeEventListener::FUNCTION_TAG, "bbb");
+  code_map.AddCode(ToAddress(0x1500), entry1, 0x200);
+  code_map.AddCode(ToAddress(0x1700), entry2, 0x100);
+  CHECK_EQ(entry1, code_map.FindEntry(ToAddress(0x1500)));
+  CHECK_EQ(entry2, code_map.FindEntry(ToAddress(0x1700)));
   code_map.MoveCode(ToAddress(0x1500), ToAddress(0x1700));  // Deprecate bbb.
   CHECK(!code_map.FindEntry(ToAddress(0x1500)));
-  CHECK_EQ(&entry1, code_map.FindEntry(ToAddress(0x1700)));
-  CodeEntry entry3(i::CodeEventListener::FUNCTION_TAG, "ccc");
-  code_map.AddCode(ToAddress(0x1750), &entry3, 0x100);
+  CHECK_EQ(entry1, code_map.FindEntry(ToAddress(0x1700)));
+  CodeEntry* entry3 = new CodeEntry(i::CodeEventListener::FUNCTION_TAG, "ccc");
+  code_map.AddCode(ToAddress(0x1750), entry3, 0x100);
   CHECK(!code_map.FindEntry(ToAddress(0x1700)));
-  CHECK_EQ(&entry3, code_map.FindEntry(ToAddress(0x1750)));
+  CHECK_EQ(entry3, code_map.FindEntry(ToAddress(0x1750)));
 }
 
-
 namespace {
 
 class TestSetup {
@@ -391,10 +389,6 @@ TEST(RecordTickSample) {
   ProfileNode* node4 = top_down_test_helper.Walk(entry1, entry3, entry1);
   CHECK(node4);
   CHECK_EQ(entry1, node4->entry());
-
-  delete entry1;
-  delete entry2;
-  delete entry3;
 }
 
 static void CheckNodeIds(const ProfileNode* node, unsigned* expectedId) {
@@ -456,10 +450,6 @@ TEST(SampleIds) {
   for (int i = 0; i < 3; i++) {
     CHECK_EQ(expected_id[i], profile->sample(i)->id());
   }
-
-  delete entry1;
-  delete entry2;
-  delete entry3;
 }
 
 
@@ -488,8 +478,6 @@ TEST(NoSamples) {
   CHECK_EQ(3u, nodeId - 1);
 
   CHECK_EQ(0, profile->samples_count());
-
-  delete entry1;
 }