[cpu-profiler] Disable logging in lazy mode when no profiles are active

Now that code entries outlive our CodeMap, it's safe to avoid storing
CodeMap metadata after the last active profiler stops. This simplifies
lifecycle logic, and avoids retaining stale data.

Bug: v8:11054
Change-Id: If30fc0835e2033b5bcca204565e05a5cba7823ea
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3000526
Commit-Queue: Andrew Comminos <acomminos@fb.com>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#75689}

src/profiler/cpu-profiler.cc

@@ -350,9 +350,6 @@ ProfilerCodeObserver::ProfilerCodeObserver(Isolate* isolate,
 void ProfilerCodeObserver::ClearCodeMap() {
   weak_code_registry_.Clear();
   code_map_.Clear();
-  // We don't currently expect any references to refcounted strings to be
-  // maintained with zero profiles after the code map is cleared.
-  DCHECK(code_entries_.strings().empty());
 }
 
 void ProfilerCodeObserver::CodeEventHandler(
@@ -504,6 +501,11 @@ CpuProfiler::~CpuProfiler() {
   GetProfilersManager()->RemoveProfiler(isolate_, this);
 
   DisableLogging();
+  profiles_.reset();
+
+  // We don't currently expect any references to refcounted strings to be
+  // maintained with zero profiles after the code map is cleared.
+  DCHECK(code_entries_.strings().empty());
 }
 
 void CpuProfiler::set_sampling_interval(base::TimeDelta value) {
@@ -519,11 +521,6 @@ void CpuProfiler::set_use_precise_sampling(bool value) {
 void CpuProfiler::ResetProfiles() {
   profiles_.reset(new CpuProfilesCollection(isolate_));
   profiles_->set_cpu_profiler(this);
-  symbolizer_.reset();
-  if (!profiling_scope_) {
-    profiler_listener_.reset();
-    code_observer_->ClearCodeMap();
-  }
 }
 
 void CpuProfiler::EnableLogging() {
@@ -543,6 +540,8 @@ void CpuProfiler::DisableLogging() {
 
   DCHECK(profiler_listener_);
   profiling_scope_.reset();
+  profiler_listener_.reset();
+  code_observer_->ClearCodeMap();
 }
 
 base::TimeDelta CpuProfiler::ComputeSamplingInterval() const {
@@ -620,9 +619,17 @@ void CpuProfiler::StartProcessorIfNotStarted() {
 
 CpuProfile* CpuProfiler::StopProfiling(const char* title) {
   if (!is_profiling_) return nullptr;
-  StopProcessorIfLastProfile(title);
+  const bool last_profile = profiles_->IsLastProfile(title);
+  if (last_profile) StopProcessor();
   CpuProfile* result = profiles_->StopProfiling(title);
+
   AdjustSamplingInterval();
+
+  DCHECK(profiling_scope_);
+  if (last_profile && logging_mode_ == kLazyLogging) {
+    DisableLogging();
+  }
+
   return result;
 }
 
@@ -630,20 +637,10 @@ CpuProfile* CpuProfiler::StopProfiling(String title) {
   return StopProfiling(profiles_->GetName(title));
 }
 
-void CpuProfiler::StopProcessorIfLastProfile(const char* title) {
-  if (!profiles_->IsLastProfile(title)) return;
-  StopProcessor();
-}
-
 void CpuProfiler::StopProcessor() {
   is_profiling_ = false;
   processor_->StopSynchronously();
   processor_.reset();
-
-  DCHECK(profiling_scope_);
-  if (logging_mode_ == kLazyLogging) {
-    DisableLogging();
-  }
 }
 }  // namespace internal
 }  // namespace v8

src/profiler/cpu-profiler.h

@@ -370,7 +370,6 @@ class V8_EXPORT_PRIVATE CpuProfiler {
 
  private:
   void StartProcessorIfNotStarted();
-  void StopProcessorIfLastProfile(const char* title);
   void StopProcessor();
   void ResetProfiles();
 

test/cctest/test-cpu-profiler.cc

@@ -306,17 +306,12 @@ TEST(CodeMapClearedBetweenProfilesWithLazyLogging) {
   CpuProfile* profile = profiler.StopProfiling("");
   CHECK(profile);
 
-  // Check that our code is still in the code map.
+  // Check that the code map is empty.
   CodeMap* code_map = profiler.code_map_for_test();
-  CodeEntry* code1_entry = code_map->FindEntry(code1->InstructionStart());
-  CHECK(code1_entry);
-  CHECK_EQ(0, strcmp("function_1", code1_entry->name()));
+  CHECK_EQ(code_map->size(), 0);
 
   profiler.DeleteProfile(profile);
 
-  // Check that the code map is emptied once the last profile is deleted.
-  CHECK(!code_map->FindEntry(code1->InstructionStart()));
-
   // Create code between profiles. This should not be logged yet.
   i::Handle<i::AbstractCode> code2(CreateCode(isolate, &env), isolate);
 
@@ -537,9 +532,12 @@ TEST(ProfileStartEndTime) {
 
 class ProfilerHelper {
  public:
-  explicit ProfilerHelper(const v8::Local<v8::Context>& context)
+  explicit ProfilerHelper(
+      const v8::Local<v8::Context>& context,
+      v8::CpuProfilingLoggingMode logging_mode = kLazyLogging)
       : context_(context),
-        profiler_(v8::CpuProfiler::New(context->GetIsolate())) {
+        profiler_(v8::CpuProfiler::New(context->GetIsolate(), kDebugNaming,
+                                       logging_mode)) {
     i::ProfilerExtension::set_profiler(profiler_);
   }
   ~ProfilerHelper() {
@@ -4194,7 +4192,7 @@ TEST(FastApiCPUProfiler) {
   // Setup and start CPU profiler.
   v8::Local<v8::Value> args[] = {
       v8::Integer::New(env->GetIsolate(), num_runs_arg)};
-  ProfilerHelper helper(env.local());
+  ProfilerHelper helper(env.local(), kEagerLogging);
   // TODO(mslekova): We could tweak the following count to reduce test
   // runtime, while still keeping the test stable.
   unsigned external_samples = 1000;