From 49d23a30213bfa627f5ed4c1a662e1087cb1786a Mon Sep 17 00:00:00 2001 From: Ali Ijaz Sheikh Date: Wed, 6 Dec 2017 15:43:09 -0800 Subject: [PATCH] deps: V8: backport 14ac02c from upstream MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Original commit message: [cpu-profiler] Clear code entries when no observers are present. 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 Reviewed-by: Yang Guo Cr-Commit-Position: refs/heads/master@{#49914} Ref: https://github.com/v8/v8/commit/14ac02c49c7a364059704537a10abae4feb15a88 PR-URL: https://github.com/nodejs/node/pull/17512 Reviewed-By: Timothy Gu Reviewed-By: Michaël Zasso Reviewed-By: Michael Dawson --- common.gypi | 2 +- deps/v8/src/profiler/profiler-listener.cc | 28 +++++++++++------------ deps/v8/src/profiler/profiler-listener.h | 3 ++- deps/v8/test/cctest/test-cpu-profiler.cc | 28 +++++++++++++++++++++++ 4 files changed, 45 insertions(+), 16 deletions(-) diff --git a/common.gypi b/common.gypi index b8e7ba833814c6..4b1f0377ba1745 100644 --- a/common.gypi +++ b/common.gypi @@ -27,7 +27,7 @@ # Reset this number to 0 on major V8 upgrades. # Increment by one for each non-official patch applied to deps/v8. - 'v8_embedder_string': '-node.11', + 'v8_embedder_string': '-node.12', # Enable disassembler for `--print-code` v8 options 'v8_enable_disassembler': 1, diff --git a/deps/v8/src/profiler/profiler-listener.cc b/deps/v8/src/profiler/profiler-listener.cc index 169b12da0770c1..540d93002459fb 100644 --- a/deps/v8/src/profiler/profiler-listener.cc +++ b/deps/v8/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); @@ -286,19 +282,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 code_entry = base::make_unique( + 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 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/deps/v8/src/profiler/profiler-listener.h b/deps/v8/src/profiler/profiler-listener.h index f4a9e24c7d00c5..440afd87a2e97f 100644 --- a/deps/v8/src/profiler/profiler-listener.h +++ b/deps/v8/src/profiler/profiler-listener.h @@ -74,6 +74,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); @@ -87,7 +88,7 @@ class ProfilerListener : public CodeEventListener { } StringsStorage function_and_resource_names_; - std::vector code_entries_; + std::vector> code_entries_; std::vector observers_; base::Mutex mutex_; diff --git a/deps/v8/test/cctest/test-cpu-profiler.cc b/deps/v8/test/cctest/test-cpu-profiler.cc index 689305f30eabc6..f22a42a977d5be 100644 --- a/deps/v8/test/cctest/test-cpu-profiler.cc +++ b/deps/v8/test/cctest/test-cpu-profiler.cc @@ -2191,3 +2191,31 @@ TEST(TracingCpuProfiler) { i::V8::SetPlatformForTesting(old_platform); } + +TEST(CodeEntriesMemoryLeak) { + v8::HandleScope scope(CcTest::isolate()); + v8::Local 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 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()); +}