[debug] Refactor initialization of the feedback vector list

This separates common logic that applies to both coverage/type profiling
(i.e. collecting feedback vectors into the list) from work that's only
required by coverage (resetting SFI::has_reported_binary_coverage and
FeedbackVector::invocation_count).

Bug: v8:6000
Change-Id: Icb36a8a6af34b3a425814d69653e331ca8f76cd5
Reviewed-on: https://chromium-review.googlesource.com/813922
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Franziska Hinkelmann <franzih@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49956}

src/debug/debug-coverage.cc

@@ -344,6 +344,16 @@ bool IsBlockMode(debug::Coverage::Mode mode) {
   }
 }
 
+bool IsBinaryMode(debug::Coverage::Mode mode) {
+  switch (mode) {
+    case debug::Coverage::kBlockBinary:
+    case debug::Coverage::kPreciseBinary:
+      return true;
+    default:
+      return false;
+  }
+}
+
 void CollectBlockCoverage(Isolate* isolate, CoverageFunction* function,
                           SharedFunctionInfo* info,
                           debug::Coverage::Mode mode) {
@@ -535,14 +545,29 @@ void Coverage::SelectMode(Isolate* isolate, debug::Coverage::Mode mode) {
     case debug::Coverage::kPreciseBinary:
     case debug::Coverage::kPreciseCount: {
       HandleScope scope(isolate);
+
       // Remove all optimized function. Optimized and inlined functions do not
       // increment invocation count.
       Deoptimizer::DeoptimizeAll(isolate);
-      if (isolate->factory()
-              ->feedback_vectors_for_profiling_tools()
-              ->IsUndefined(isolate)) {
-        isolate->InitializeVectorListFromHeap();
+
+      // Root all feedback vectors to avoid early collection.
+      isolate->MaybeInitializeVectorListFromHeap();
+
+      HeapIterator heap_iterator(isolate->heap());
+      while (HeapObject* o = heap_iterator.next()) {
+        if (IsBinaryMode(mode) && o->IsSharedFunctionInfo()) {
+          // If collecting binary coverage, reset
+          // SFI::has_reported_binary_coverage to avoid optimizing / inlining
+          // functions before they have reported coverage.
+          SharedFunctionInfo* shared = SharedFunctionInfo::cast(o);
+          shared->set_has_reported_binary_coverage(false);
+        } else if (o->IsFeedbackVector()) {
+          // In any case, clear any collected invocation counts.
+          FeedbackVector* vector = FeedbackVector::cast(o);
+          vector->clear_invocation_count();
+        }
       }
+
       break;
     }
   }

src/debug/debug-type-profile.cc

@@ -105,10 +105,7 @@ void TypeProfile::SelectMode(Isolate* isolate, debug::TypeProfile::Mode mode) {
     }
   } else {
     DCHECK_EQ(debug::TypeProfile::Mode::kCollect, mode);
-    if (isolate->factory()->feedback_vectors_for_profiling_tools()->IsUndefined(
-            isolate)) {
-      isolate->InitializeVectorListFromHeap();
-    }
+    isolate->MaybeInitializeVectorListFromHeap();
   }
   isolate->set_type_profile_mode(mode);
 }

src/isolate.cc

@@ -3181,27 +3181,32 @@ void Isolate::SetFeedbackVectorsForProfilingTools(Object* value) {
   heap()->set_feedback_vectors_for_profiling_tools(value);
 }
 
-void Isolate::InitializeVectorListFromHeap() {
+void Isolate::MaybeInitializeVectorListFromHeap() {
+  if (!heap()->feedback_vectors_for_profiling_tools()->IsUndefined(this)) {
+    // Already initialized, return early.
+    DCHECK(heap()->feedback_vectors_for_profiling_tools()->IsArrayList());
+    return;
+  }
+
   // Collect existing feedback vectors.
   std::vector<Handle<FeedbackVector>> vectors;
+
   {
     HeapIterator heap_iterator(heap());
     while (HeapObject* current_obj = heap_iterator.next()) {
-      if (current_obj->IsSharedFunctionInfo()) {
-        SharedFunctionInfo* shared = SharedFunctionInfo::cast(current_obj);
-        shared->set_has_reported_binary_coverage(false);
-      } else if (current_obj->IsFeedbackVector()) {
-        FeedbackVector* vector = FeedbackVector::cast(current_obj);
-        SharedFunctionInfo* shared = vector->shared_function_info();
-        if (!shared->IsSubjectToDebugging()) continue;
-        vector->clear_invocation_count();
-        vectors.emplace_back(vector, this);
-      }
+      if (!current_obj->IsFeedbackVector()) continue;
+
+      FeedbackVector* vector = FeedbackVector::cast(current_obj);
+      SharedFunctionInfo* shared = vector->shared_function_info();
+
+      // No need to preserve the feedback vector for non-user-visible functions.
+      if (!shared->IsSubjectToDebugging()) continue;
+
+      vectors.emplace_back(vector, this);
     }
   }
 
-  // Add collected feedback vectors to the root list lest we lose them to
-  // GC.
+  // Add collected feedback vectors to the root list lest we lose them to GC.
   Handle<ArrayList> list =
       ArrayList::New(this, static_cast<int>(vectors.size()));
   for (const auto& vector : vectors) list = ArrayList::Add(list, vector);

src/isolate.h

@@ -1084,7 +1084,7 @@ class Isolate {
   // memory usage is expected.
   void SetFeedbackVectorsForProfilingTools(Object* value);
 
-  void InitializeVectorListFromHeap();
+  void MaybeInitializeVectorListFromHeap();
 
   double time_millis_since_init() {
     return heap_.MonotonicallyIncreasingTimeInMs() - time_millis_at_init_;