[cpu-profiler] Clear code entries when no observers are present.

a1ph · Commit Bot · commit 14ac02c49c7a · 2017-12-06T22:58:05.000Z
Performed manual testing as well by making 20 CPU profile recordings of loading http://meduza.io page. Without the patch the page renderer memory size grows beyond 300MB. With the patch it remains below 200MB. BUG=v8:6623 Change-Id: Ifce541b84bb2aaaa5175520f8dd49dbc0cb5dd20 Reviewed-on: https://chromium-review.googlesource.com/798020 Commit-Queue: Alexei Filippov <alph@chromium.org> Reviewed-by: Yang Guo <yangguo@chromium.org> Cr-Commit-Position: refs/heads/master@{#49914}
diff --git a/src/profiler/profiler-listener.cc b/src/profiler/profiler-listener.cc
@@ -16,11 +16,7 @@ namespace internal {
 ProfilerListener::ProfilerListener(Isolate* isolate)
     : function_and_resource_names_(isolate->heap()) {}
 
-ProfilerListener::~ProfilerListener() {
-  for (auto code_entry : code_entries_) {
-    delete code_entry;
-  }
-}
+ProfilerListener::~ProfilerListener() = default;
 
 void ProfilerListener::CallbackEvent(Name* name, Address entry_point) {
   CodeEventsContainer evt_rec(CodeEventRecord::CODE_CREATION);
@@ -274,19 +270,23 @@ CodeEntry* ProfilerListener::NewCodeEntry(
     CodeEventListener::LogEventsAndTags tag, const char* name,
     const char* name_prefix, const char* resource_name, int line_number,
     int column_number, JITLineInfoTable* line_info, Address instruction_start) {
-  CodeEntry* code_entry =
-      new CodeEntry(tag, name, name_prefix, resource_name, line_number,
-                    column_number, line_info, instruction_start);
-  code_entries_.push_back(code_entry);
-  return code_entry;
+  std::unique_ptr<CodeEntry> code_entry = base::make_unique<CodeEntry>(
+      tag, name, name_prefix, resource_name, line_number, column_number,
+      line_info, instruction_start);
+  CodeEntry* raw_code_entry = code_entry.get();
+  code_entries_.push_back(std::move(code_entry));
+  return raw_code_entry;
 }
 
 void ProfilerListener::AddObserver(CodeEventObserver* observer) {
   base::LockGuard<base::Mutex> guard(&mutex_);
-  if (std::find(observers_.begin(), observers_.end(), observer) !=
-      observers_.end())
-    return;
-  observers_.push_back(observer);
+  if (observers_.empty()) {
+    code_entries_.clear();
+  }
+  if (std::find(observers_.begin(), observers_.end(), observer) ==
+      observers_.end()) {
+    observers_.push_back(observer);
+  }
 }
 
 void ProfilerListener::RemoveObserver(CodeEventObserver* observer) {
diff --git a/src/profiler/profiler-listener.h b/src/profiler/profiler-listener.h
@@ -73,6 +73,7 @@ class ProfilerListener : public CodeEventListener {
   const char* GetFunctionName(const char* name) {
     return function_and_resource_names_.GetFunctionName(name);
   }
+  size_t entries_count_for_test() const { return code_entries_.size(); }
 
  private:
   void RecordInliningInfo(CodeEntry* entry, AbstractCode* abstract_code);
@@ -86,7 +87,7 @@ class ProfilerListener : public CodeEventListener {
   }
 
   StringsStorage function_and_resource_names_;
-  std::vector<CodeEntry*> code_entries_;
+  std::vector<std::unique_ptr<CodeEntry>> code_entries_;
   std::vector<CodeEventObserver*> observers_;
   base::Mutex mutex_;
 
diff --git a/test/cctest/test-cpu-profiler.cc b/test/cctest/test-cpu-profiler.cc
@@ -2267,6 +2267,34 @@ TEST(StaticCollectSampleAPI) {
   profile->Delete();
 }
 
+TEST(CodeEntriesMemoryLeak) {
+  v8::HandleScope scope(CcTest::isolate());
+  v8::Local<v8::Context> env = CcTest::NewContext(PROFILER_EXTENSION);
+  v8::Context::Scope context_scope(env);
+
+  std::string source = "function start() {}\n";
+  for (int i = 0; i < 1000; ++i) {
+    source += "function foo" + std::to_string(i) + "() { return " +
+              std::to_string(i) +
+              "; }\n"
+              "foo" +
+              std::to_string(i) + "();\n";
+  }
+  CompileRun(source.c_str());
+  v8::Local<v8::Function> function = GetFunction(env, "start");
+
+  ProfilerHelper helper(env);
+
+  for (int j = 0; j < 100; ++j) {
+    v8::CpuProfile* profile = helper.Run(function, nullptr, 0);
+    profile->Delete();
+  }
+  ProfilerListener* profiler_listener =
+      CcTest::i_isolate()->logger()->profiler_listener();
+
+  CHECK_GE(10000ul, profiler_listener->entries_count_for_test());
+}
+
 }  // namespace test_cpu_profiler
 }  // namespace internal
 }  // namespace v8

Original file line number	Diff line number	Diff line change
`@@ -73,6 +73,7 @@ class ProfilerListener : public CodeEventListener {`
`73`	`73`	`const char* GetFunctionName(const char* name) {`
`74`	`74`	`return function_and_resource_names_.GetFunctionName(name);`
`75`	`75`	`}`
	`76`	`+ size_t entries_count_for_test() const { return code_entries_.size(); }`
`76`	`77`
`77`	`78`	`private:`
`78`	`79`	`void RecordInliningInfo(CodeEntry* entry, AbstractCode* abstract_code);`
`@@ -86,7 +87,7 @@ class ProfilerListener : public CodeEventListener {`
`86`	`87`	`}`
`87`	`88`
`88`	`89`	`StringsStorage function_and_resource_names_;`
`89`		`- std::vector<CodeEntry*> code_entries_;`
	`90`	`+ std::vector<std::unique_ptr<CodeEntry>> code_entries_;`
`90`	`91`	`std::vector<CodeEventObserver*> observers_;`
`91`	`92`	`base::Mutex mutex_;`
`92`	`93`