[type-profile] Use vector list from isolate

Instead of re-iterating over the heap all the time, use the
list of feedback vectors on the isolate. This also avoids GC of vectors.

Bug: v8:5935
Change-Id: I0bb96fcf2b0feb9856e9806f812188de1fc7b37e
Reviewed-on: https://chromium-review.googlesource.com/668396
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Franziska Hinkelmann <franzih@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48643}

src/debug/debug-coverage.cc

@@ -382,11 +382,12 @@ std::unique_ptr<Coverage> Coverage::CollectPrecise(Isolate* isolate) {
   DCHECK(!isolate->is_best_effort_code_coverage());
   std::unique_ptr<Coverage> result =
       Collect(isolate, isolate->code_coverage_mode());
-  if (isolate->is_precise_binary_code_coverage() ||
+  if (!isolate->is_collecting_type_profile() &&
-      isolate->is_block_binary_code_coverage()) {
+      (isolate->is_precise_binary_code_coverage() ||
+       isolate->is_block_binary_code_coverage())) {
     // We do not have to hold onto feedback vectors for invocations we already
     // reported. So we can reset the list.
-    isolate->SetCodeCoverageList(*ArrayList::New(isolate, 0));
+    isolate->SetFeedbackVectorsForProfilingTools(*ArrayList::New(isolate, 0));
   }
   return result;
 }
@@ -407,9 +408,11 @@ std::unique_ptr<Coverage> Coverage::Collect(
     case v8::debug::Coverage::kPreciseBinary:
     case v8::debug::Coverage::kPreciseCount: {
       // Feedback vectors are already listed to prevent losing them to GC.
-      DCHECK(isolate->factory()->code_coverage_list()->IsArrayList());
+      DCHECK(isolate->factory()
-      Handle<ArrayList> list =
+                 ->feedback_vectors_for_profiling_tools()
-          Handle<ArrayList>::cast(isolate->factory()->code_coverage_list());
+                 ->IsArrayList());
+      Handle<ArrayList> list = Handle<ArrayList>::cast(
+          isolate->factory()->feedback_vectors_for_profiling_tools());
       for (int i = 0; i < list->Length(); i++) {
         FeedbackVector* vector = FeedbackVector::cast(list->Get(i));
         SharedFunctionInfo* shared = vector->shared_function_info();
@@ -421,7 +424,9 @@ std::unique_ptr<Coverage> Coverage::Collect(
       break;
     }
     case v8::debug::Coverage::kBestEffort: {
-      DCHECK(!isolate->factory()->code_coverage_list()->IsArrayList());
+      DCHECK(!isolate->factory()
+                  ->feedback_vectors_for_profiling_tools()
+                  ->IsArrayList());
       DCHECK_EQ(v8::debug::Coverage::kBestEffort, collectionMode);
       HeapIterator heap_iterator(isolate->heap());
       while (HeapObject* current_obj = heap_iterator.next()) {
@@ -520,7 +525,10 @@ void Coverage::SelectMode(Isolate* isolate, debug::Coverage::Mode mode) {
       // following coverage recording (without reloads) will be at function
       // granularity.
       isolate->debug()->RemoveAllCoverageInfos();
-      isolate->SetCodeCoverageList(isolate->heap()->undefined_value());
+      if (!isolate->is_collecting_type_profile()) {
+        isolate->SetFeedbackVectorsForProfilingTools(
+            isolate->heap()->undefined_value());
+      }
       break;
     case debug::Coverage::kBlockBinary:
     case debug::Coverage::kBlockCount:
@@ -530,32 +538,11 @@ void Coverage::SelectMode(Isolate* isolate, debug::Coverage::Mode mode) {
       // Remove all optimized function. Optimized and inlined functions do not
       // increment invocation count.
       Deoptimizer::DeoptimizeAll(isolate);
-      // Collect existing feedback vectors.
+      if (isolate->factory()
-      std::vector<Handle<FeedbackVector>> vectors;
+              ->feedback_vectors_for_profiling_tools()
-      {
+              ->IsUndefined(isolate)) {
-        HeapIterator heap_iterator(isolate->heap());
+        isolate->InitializeVectorListFromHeap();
-        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, isolate);
-          } else if (current_obj->IsJSFunction()) {
-            JSFunction* function = JSFunction::cast(current_obj);
-            function->set_code(function->shared()->code());
-          }
-        }
       }
-      // Add collected feedback vectors to the root list lest we lose them to
-      // GC.
-      Handle<ArrayList> list =
-          ArrayList::New(isolate, static_cast<int>(vectors.size()));
-      for (const auto& vector : vectors) list = ArrayList::Add(list, vector);
-      isolate->SetCodeCoverageList(*list);
       break;
     }
   }

src/debug/debug-type-profile.cc

@@ -15,19 +15,12 @@ namespace internal {
 std::unique_ptr<TypeProfile> TypeProfile::Collect(Isolate* isolate) {
   std::unique_ptr<TypeProfile> result(new TypeProfile());
 
-  // Collect existing feedback vectors.
+  // Feedback vectors are already listed to prevent losing them to GC.
-  std::vector<Handle<FeedbackVector>> feedback_vectors;
+  DCHECK(isolate->factory()
-  {
+             ->feedback_vectors_for_profiling_tools()
-    HeapIterator heap_iterator(isolate->heap());
+             ->IsArrayList());
-    while (HeapObject* current_obj = heap_iterator.next()) {
+  Handle<ArrayList> list = Handle<ArrayList>::cast(
-      if (current_obj->IsFeedbackVector()) {
+      isolate->factory()->feedback_vectors_for_profiling_tools());
-        FeedbackVector* vector = FeedbackVector::cast(current_obj);
-        SharedFunctionInfo* shared = vector->shared_function_info();
-        if (!shared->IsSubjectToDebugging()) continue;
-        feedback_vectors.emplace_back(vector, isolate);
-      }
-    }
-  }
 
   Script::Iterator scripts(isolate);
 
@@ -41,7 +34,10 @@ std::unique_ptr<TypeProfile> TypeProfile::Collect(Isolate* isolate) {
     TypeProfileScript type_profile_script(script_handle);
     std::vector<TypeProfileEntry>* entries = &type_profile_script.entries;
 
-    for (const auto& vector : feedback_vectors) {
+    // TODO(franzih): Sort the vectors by script first instead of iterating
+    // the list multiple times.
+    for (int i = 0; i < list->Length(); i++) {
+      FeedbackVector* vector = FeedbackVector::cast(list->Get(i));
       SharedFunctionInfo* info = vector->shared_function_info();
       DCHECK(info->IsSubjectToDebugging());
 
@@ -74,28 +70,47 @@ std::unique_ptr<TypeProfile> TypeProfile::Collect(Isolate* isolate) {
 }
 
 void TypeProfile::SelectMode(Isolate* isolate, debug::TypeProfile::Mode mode) {
-  isolate->set_type_profile_mode(mode);
   HandleScope handle_scope(isolate);
 
   if (mode == debug::TypeProfile::Mode::kNone) {
-    // Release type profile data collected so far.
+    if (!isolate->factory()
-    {
+             ->feedback_vectors_for_profiling_tools()
-      HeapIterator heap_iterator(isolate->heap());
+             ->IsUndefined(isolate)) {
-      while (HeapObject* current_obj = heap_iterator.next()) {
+      // Release type profile data collected so far.
-        if (current_obj->IsFeedbackVector()) {
+
-          FeedbackVector* vector = FeedbackVector::cast(current_obj);
+      // Feedback vectors are already listed to prevent losing them to GC.
-          SharedFunctionInfo* info = vector->shared_function_info();
+      DCHECK(isolate->factory()
-          if (!info->IsSubjectToDebugging() ||
+                 ->feedback_vectors_for_profiling_tools()
-              info->feedback_metadata()->is_empty() ||
+                 ->IsArrayList());
-              !info->feedback_metadata()->HasTypeProfileSlot())
+      Handle<ArrayList> list = Handle<ArrayList>::cast(
-            continue;
+          isolate->factory()->feedback_vectors_for_profiling_tools());
+
+      for (int i = 0; i < list->Length(); i++) {
+        FeedbackVector* vector = FeedbackVector::cast(list->Get(i));
+        SharedFunctionInfo* info = vector->shared_function_info();
+        DCHECK(info->IsSubjectToDebugging());
+        if (info->feedback_metadata()->HasTypeProfileSlot()) {
           FeedbackSlot slot = vector->GetTypeProfileSlot();
           CollectTypeProfileNexus nexus(vector, slot);
           nexus.Clear();
         }
       }
+
+      // Delete the feedback vectors from the list if they're not used by code
+      // coverage.
+      if (isolate->is_best_effort_code_coverage()) {
+        isolate->SetFeedbackVectorsForProfilingTools(
+            isolate->heap()->undefined_value());
+      }
+    }
+  } else {
+    DCHECK_EQ(debug::TypeProfile::Mode::kCollect, mode);
+    if (isolate->factory()->feedback_vectors_for_profiling_tools()->IsUndefined(
+            isolate)) {
+      isolate->InitializeVectorListFromHeap();
     }
   }
+  isolate->set_type_profile_mode(mode);
 }
 
 }  // namespace internal

src/feedback-vector.cc

@@ -297,8 +297,9 @@ Handle<FeedbackVector> FeedbackVector::New(Isolate* isolate,
   }
 
   Handle<FeedbackVector> result = Handle<FeedbackVector>::cast(vector);
-  if (!isolate->is_best_effort_code_coverage()) {
+  if (!isolate->is_best_effort_code_coverage() ||
-    AddToCodeCoverageList(isolate, result);
+      isolate->is_collecting_type_profile()) {
+    AddToVectorsForProfilingTools(isolate, result);
   }
   return result;
 }
@@ -309,21 +310,23 @@ Handle<FeedbackVector> FeedbackVector::Copy(Isolate* isolate,
   Handle<FeedbackVector> result;
   result = Handle<FeedbackVector>::cast(
       isolate->factory()->CopyFixedArray(Handle<FixedArray>::cast(vector)));
-  if (!isolate->is_best_effort_code_coverage()) {
+  if (!isolate->is_best_effort_code_coverage() ||
-    AddToCodeCoverageList(isolate, result);
+      isolate->is_collecting_type_profile()) {
+    AddToVectorsForProfilingTools(isolate, result);
   }
   return result;
 }
 
 // static
-void FeedbackVector::AddToCodeCoverageList(Isolate* isolate,
+void FeedbackVector::AddToVectorsForProfilingTools(
-                                           Handle<FeedbackVector> vector) {
+    Isolate* isolate, Handle<FeedbackVector> vector) {
-  DCHECK(!isolate->is_best_effort_code_coverage());
+  DCHECK(!isolate->is_best_effort_code_coverage() ||
+         isolate->is_collecting_type_profile());
   if (!vector->shared_function_info()->IsSubjectToDebugging()) return;
-  Handle<ArrayList> list =
+  Handle<ArrayList> list = Handle<ArrayList>::cast(
-      Handle<ArrayList>::cast(isolate->factory()->code_coverage_list());
+      isolate->factory()->feedback_vectors_for_profiling_tools());
   list = ArrayList::Add(list, vector);
-  isolate->SetCodeCoverageList(*list);
+  isolate->SetFeedbackVectorsForProfilingTools(*list);
 }
 
 // static

src/feedback-vector.h

@@ -278,8 +278,8 @@ class FeedbackVector : public HeapObject {
   }
 
  private:
-  static void AddToCodeCoverageList(Isolate* isolate,
+  static void AddToVectorsForProfilingTools(Isolate* isolate,
-                                    Handle<FeedbackVector> vector);
+                                            Handle<FeedbackVector> vector);
 
   DISALLOW_IMPLICIT_CONSTRUCTORS(FeedbackVector);
 };

src/heap/heap.cc

@@ -2559,7 +2559,7 @@ bool Heap::RootCanBeWrittenAfterInitialization(Heap::RootListIndex root_index) {
     case kWeakNewSpaceObjectToCodeListRootIndex:
     case kRetainedMapsRootIndex:
     case kRetainingPathTargetsRootIndex:
-    case kCodeCoverageListRootIndex:
+    case kFeedbackVectorsForProfilingToolsRootIndex:
     case kNoScriptSharedFunctionInfosRootIndex:
     case kWeakStackTraceListRootIndex:
     case kSerializedTemplatesRootIndex:

src/heap/heap.h

@@ -227,8 +227,9 @@ using v8::MemoryPressureLevel;
   /* slots refer to the code with the reference to the weak object. */         \
   V(ArrayList, weak_new_space_object_to_code_list,                             \
     WeakNewSpaceObjectToCodeList)                                              \
-  /* List to hold onto feedback vectors that we need for code coverage */      \
+  /* Feedback vectors that we need for code coverage or type profile */        \
-  V(Object, code_coverage_list, CodeCoverageList)                              \
+  V(Object, feedback_vectors_for_profiling_tools,                              \
+    FeedbackVectorsForProfilingTools)                                          \
   V(Object, weak_stack_trace_list, WeakStackTraceList)                         \
   V(Object, noscript_shared_function_infos, NoScriptSharedFunctionInfos)       \
   V(FixedArray, serialized_templates, SerializedTemplates)                     \

src/heap/setup-heap-internal.cc

@@ -542,7 +542,7 @@ void Heap::CreateInitialObjects() {
       ArrayList::cast(*(factory->NewFixedArray(16, TENURED))));
   weak_new_space_object_to_code_list()->SetLength(0);
 
-  set_code_coverage_list(undefined_value());
+  set_feedback_vectors_for_profiling_tools(undefined_value());
 
   set_script_list(Smi::kZero);
 

src/isolate.cc

@@ -3030,9 +3030,39 @@ bool Isolate::NeedsSourcePositionsForProfiling() const {
          debug_->is_active() || logger_->is_logging();
 }
 
-void Isolate::SetCodeCoverageList(Object* value) {
+void Isolate::SetFeedbackVectorsForProfilingTools(Object* value) {
   DCHECK(value->IsUndefined(this) || value->IsArrayList());
-  heap()->set_code_coverage_list(value);
+  heap()->set_feedback_vectors_for_profiling_tools(value);
+}
+
+void Isolate::InitializeVectorListFromHeap() {
+  // 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);
+      } else if (current_obj->IsJSFunction()) {
+        JSFunction* function = JSFunction::cast(current_obj);
+        function->set_code(function->shared()->code());
+      }
+    }
+  }
+
+  // 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);
+  SetFeedbackVectorsForProfilingTools(*list);
 }
 
 bool Isolate::IsArrayOrObjectPrototype(Object* object) {

src/isolate.h

@@ -1056,7 +1056,14 @@ class Isolate {
     return type_profile_mode() == debug::TypeProfile::kCollect;
   }
 
-  void SetCodeCoverageList(Object* value);
+  // Collect feedback vectors with data for code coverage or type profile.
+  // Reset the list, when both code coverage and type profile are not
+  // needed anymore. This keeps many feedback vectors alive, but code
+  // coverage or type profile are used for debugging only and increase in
+  // memory usage is expected.
+  void SetFeedbackVectorsForProfilingTools(Object* value);
+
+  void InitializeVectorListFromHeap();
 
   double time_millis_since_init() {
     return heap_.MonotonicallyIncreasingTimeInMs() - time_millis_at_init_;

test/inspector/type-profiler/type-profile-start-stop-expected.txt

@@ -47,3 +47,5 @@ function f(/*null*/a) {
   return 'second';
 /*string*/};
 f(null);
+
+Running test: testStopTwice

test/inspector/type-profiler/type-profile-start-stop.js

@@ -166,5 +166,13 @@ InspectorTest.runAsyncTestSuite([
     Protocol.Profiler.stopTypeProfile();
     Protocol.Profiler.disable();
     await Protocol.Runtime.disable();
-  }
+  },
+  async function testStopTwice() {
+    Protocol.Runtime.enable();
+    await Protocol.Profiler.enable();
+    await Protocol.Profiler.stopTypeProfile();
+    await Protocol.Profiler.stopTypeProfile();
+    Protocol.Profiler.disable();
+    await Protocol.Runtime.disable();
+  },
 ]);