Reland "[GC] Ensure JSFunctions with flushed bytecode are flushed during GC."

This is a reland of f5729f1cda

TBR=ulan@chromium.org

Original change's description:
> [GC] Ensure JSFunctions with flushed bytecode are flushed during GC.
>
> When bytecode is flushed from a SFI, the JSFunctions still retain their
> FeedbackVector's and point to the interpreter entry trampoline. They are
> reset if re-executed, however if not they could hold onto the feedback
> vector indefinetly. This CL adds a pass the GC to detect JSFunctions that
> need to be reset, and performs the reset at the end of GC.
>
> BUG=v8:8395
>
> Change-Id: I3de8655aff9ff80f912b4fd51dee43eb98cfd519
> Reviewed-on: https://chromium-review.googlesource.com/c/1393292
> Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
> Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#58775}

Bug: v8:8395
Change-Id: If9580b25ba32e4065e20d86cb8ed22a3280d59e9
Reviewed-on: https://chromium-review.googlesource.com/c/1424860
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59001}

src/heap-symbols.h

@@ -383,6 +383,7 @@
   TOP_MC_SCOPES(F)                                   \
   F(MC_CLEAR_DEPENDENT_CODE)                         \
   F(MC_CLEAR_FLUSHABLE_BYTECODE)                     \
+  F(MC_CLEAR_FLUSHED_JS_FUNCTIONS)                   \
   F(MC_CLEAR_MAPS)                                   \
   F(MC_CLEAR_SLOTS_BUFFER)                           \
   F(MC_CLEAR_STORE_BUFFER)                           \

src/heap/concurrent-marking.cc

@@ -387,6 +387,17 @@ class ConcurrentMarkingVisitor final
     return size;
   }
 
+  int VisitJSFunction(Map map, JSFunction object) {
+    int size = VisitJSObjectSubclass(map, object);
+
+    // Check if the JSFunction needs reset due to bytecode being flushed.
+    if (object->NeedsResetDueToFlushedBytecode()) {
+      weak_objects_->flushed_js_functions.Push(task_id_, object);
+    }
+
+    return size;
+  }
+
   int VisitMap(Map meta_map, Map map) {
     if (!ShouldVisit(map)) return 0;
     int size = Map::BodyDescriptor::SizeOf(meta_map, map);
@@ -825,6 +836,7 @@ void ConcurrentMarking::Run(int task_id, TaskState* task_state) {
     weak_objects_->js_weak_cells.FlushToGlobal(task_id);
     weak_objects_->weak_objects_in_code.FlushToGlobal(task_id);
     weak_objects_->bytecode_flushing_candidates.FlushToGlobal(task_id);
+    weak_objects_->flushed_js_functions.FlushToGlobal(task_id);
     base::AsAtomicWord::Relaxed_Store<size_t>(&task_state->marked_bytes, 0);
     total_marked_bytes_ += marked_bytes;
 

src/heap/mark-compact-inl.h

@@ -93,6 +93,19 @@ int MarkingVisitor<fixed_array_mode, retaining_path_mode, MarkingState>::
   return size;
 }
 
+template <FixedArrayVisitationMode fixed_array_mode,
+          TraceRetainingPathMode retaining_path_mode, typename MarkingState>
+int MarkingVisitor<fixed_array_mode, retaining_path_mode,
+                   MarkingState>::VisitJSFunction(Map map, JSFunction object) {
+  int size = Parent::VisitJSFunction(map, object);
+
+  // Check if the JSFunction needs reset due to bytecode being flushed.
+  if (FLAG_flush_bytecode && object->NeedsResetDueToFlushedBytecode()) {
+    collector_->AddFlushedJSFunction(object);
+  }
+
+  return size;
+}
 template <FixedArrayVisitationMode fixed_array_mode,
           TraceRetainingPathMode retaining_path_mode, typename MarkingState>
 int MarkingVisitor<fixed_array_mode, retaining_path_mode,
@@ -486,6 +499,10 @@ void MarkCompactCollector::AddBytecodeFlushingCandidate(
   weak_objects_.bytecode_flushing_candidates.Push(kMainThread, flush_candidate);
 }
 
+void MarkCompactCollector::AddFlushedJSFunction(JSFunction flushed_function) {
+  weak_objects_.flushed_js_functions.Push(kMainThread, flushed_function);
+}
+
 template <LiveObjectIterationMode mode>
 LiveObjectRange<mode>::iterator::iterator(MemoryChunk* chunk, Bitmap* bitmap,
                                           Address start)

src/heap/mark-compact.cc

@@ -1905,6 +1905,11 @@ void MarkCompactCollector::ClearNonLiveReferences() {
     ClearOldBytecodeCandidates();
   }
 
+  {
+    TRACE_GC(heap()->tracer(), GCTracer::Scope::MC_CLEAR_FLUSHED_JS_FUNCTIONS);
+    ClearFlushedJsFunctions();
+  }
+
   {
     TRACE_GC(heap()->tracer(), GCTracer::Scope::MC_CLEAR_WEAK_LISTS);
     // Process the weak references.
@@ -1934,6 +1939,7 @@ void MarkCompactCollector::ClearNonLiveReferences() {
   DCHECK(weak_objects_.js_weak_refs.IsEmpty());
   DCHECK(weak_objects_.js_weak_cells.IsEmpty());
   DCHECK(weak_objects_.bytecode_flushing_candidates.IsEmpty());
+  DCHECK(weak_objects_.flushed_js_functions.IsEmpty());
 }
 
 void MarkCompactCollector::MarkDependentCodeForDeoptimization() {
@@ -2072,6 +2078,15 @@ void MarkCompactCollector::ClearOldBytecodeCandidates() {
   }
 }
 
+void MarkCompactCollector::ClearFlushedJsFunctions() {
+  DCHECK(FLAG_flush_bytecode || weak_objects_.flushed_js_functions.IsEmpty());
+  JSFunction flushed_js_function;
+  while (weak_objects_.flushed_js_functions.Pop(kMainThread,
+                                                &flushed_js_function)) {
+    flushed_js_function->ResetIfBytecodeFlushed();
+  }
+}
+
 void MarkCompactCollector::ClearFullMapTransitions() {
   TransitionArray array;
   while (weak_objects_.transition_arrays.Pop(kMainThread, &array)) {
@@ -2329,6 +2344,7 @@ void MarkCompactCollector::AbortWeakObjects() {
   weak_objects_.js_weak_refs.Clear();
   weak_objects_.js_weak_cells.Clear();
   weak_objects_.bytecode_flushing_candidates.Clear();
+  weak_objects_.flushed_js_functions.Clear();
 }
 
 bool MarkCompactCollector::IsOnEvacuationCandidate(MaybeObject obj) {

src/heap/mark-compact.h

@@ -449,6 +449,7 @@ struct WeakObjects {
   Worklist<JSWeakCell, 64> js_weak_cells;
 
   Worklist<SharedFunctionInfo, 64> bytecode_flushing_candidates;
+  Worklist<JSFunction, 64> flushed_js_functions;
 };
 
 struct EphemeronMarking {
@@ -662,6 +663,7 @@ class MarkCompactCollector final : public MarkCompactCollectorBase {
   }
 
   inline void AddBytecodeFlushingCandidate(SharedFunctionInfo flush_candidate);
+  inline void AddFlushedJSFunction(JSFunction flushed_function);
 
   void AddNewlyDiscovered(HeapObject object) {
     if (ephemeron_marking_.newly_discovered_overflowed) return;
@@ -793,6 +795,9 @@ class MarkCompactCollector final : public MarkCompactCollectorBase {
   // collections.
   void ClearOldBytecodeCandidates();
 
+  // Resets any JSFunctions which have had their bytecode flushed.
+  void ClearFlushedJsFunctions();
+
   // Compact every array in the global list of transition arrays and
   // trim the corresponding descriptor array if a transition target is non-live.
   void ClearFullMapTransitions();
@@ -927,6 +932,7 @@ class MarkingVisitor final
   V8_INLINE int VisitFixedArray(Map map, FixedArray object);
   V8_INLINE int VisitJSApiObject(Map map, JSObject object);
   V8_INLINE int VisitJSArrayBuffer(Map map, JSArrayBuffer object);
+  V8_INLINE int VisitJSFunction(Map map, JSFunction object);
   V8_INLINE int VisitJSDataView(Map map, JSDataView object);
   V8_INLINE int VisitJSTypedArray(Map map, JSTypedArray object);
   V8_INLINE int VisitMap(Map map, Map object);

src/heap/objects-visiting.h

@@ -55,6 +55,7 @@ class WasmInstanceObject;
   V(FixedTypedArrayBase, FixedTypedArrayBase)                             \
   V(JSArrayBuffer, JSArrayBuffer)                                         \
   V(JSDataView, JSDataView)                                               \
+  V(JSFunction, JSFunction)                                               \
   V(JSObject, JSObject)                                                   \
   V(JSTypedArray, JSTypedArray)                                           \
   V(JSWeakCell, JSWeakCell)                                               \

src/objects-body-descriptors-inl.h

@@ -197,30 +197,6 @@ class JSObject::FastBodyDescriptor final : public BodyDescriptorBase {
   }
 };
 
-class JSFunction::BodyDescriptor final : public BodyDescriptorBase {
- public:
-  static bool IsValidSlot(Map map, HeapObject obj, int offset) {
-    if (offset < kSizeWithoutPrototype) return true;
-    if (offset < kSizeWithPrototype && map->has_prototype_slot()) {
-      return true;
-    }
-    return IsValidJSObjectSlotImpl(map, obj, offset);
-  }
-
-  template <typename ObjectVisitor>
-  static inline void IterateBody(Map map, HeapObject obj, int object_size,
-                                 ObjectVisitor* v) {
-    int header_size = JSFunction::GetHeaderSize(map->has_prototype_slot());
-    DCHECK_EQ(header_size, JSObject::GetHeaderSize(map));
-    IteratePointers(obj, kPropertiesOrHashOffset, header_size, v);
-    IterateJSObjectBodyImpl(map, obj, header_size, object_size, v);
-  }
-
-  static inline int SizeOf(Map map, HeapObject object) {
-    return map->instance_size();
-  }
-};
-
 class JSWeakCell::BodyDescriptor final : public BodyDescriptorBase {
  public:
   static bool IsValidSlot(Map map, HeapObject obj, int offset) {

src/objects.cc

@@ -3220,6 +3220,9 @@ VisitorId Map::GetVisitorId(Map map) {
     case JS_DATA_VIEW_TYPE:
       return kVisitJSDataView;
 
+    case JS_FUNCTION_TYPE:
+      return kVisitJSFunction;
+
     case JS_TYPED_ARRAY_TYPE:
       return kVisitJSTypedArray;
 
@@ -3260,7 +3263,6 @@ VisitorId Map::GetVisitorId(Map map) {
     case JS_DATE_TYPE:
     case JS_ARRAY_ITERATOR_TYPE:
     case JS_ARRAY_TYPE:
-    case JS_FUNCTION_TYPE:
     case JS_GLOBAL_PROXY_TYPE:
     case JS_GLOBAL_OBJECT_TYPE:
     case JS_MESSAGE_OBJECT_TYPE:

src/objects/js-objects-inl.h

@@ -459,7 +459,6 @@ ACCESSORS(JSBoundFunction, bound_target_function, JSReceiver,
 ACCESSORS(JSBoundFunction, bound_this, Object, kBoundThisOffset)
 ACCESSORS(JSBoundFunction, bound_arguments, FixedArray, kBoundArgumentsOffset)
 
-ACCESSORS(JSFunction, shared, SharedFunctionInfo, kSharedFunctionInfoOffset)
 ACCESSORS(JSFunction, raw_feedback_cell, FeedbackCell, kFeedbackCellOffset)
 
 ACCESSORS(JSGlobalObject, native_context, Context, kNativeContextOffset)
@@ -542,18 +541,29 @@ AbstractCode JSFunction::abstract_code() {
 }
 
 Code JSFunction::code() const {
-  return Code::cast(READ_FIELD(*this, kCodeOffset));
+  return Code::cast(RELAXED_READ_FIELD(*this, kCodeOffset));
 }
 
 void JSFunction::set_code(Code value) {
   DCHECK(!Heap::InNewSpace(value));
-  WRITE_FIELD(*this, kCodeOffset, value);
+  RELAXED_WRITE_FIELD(*this, kCodeOffset, value);
   MarkingBarrier(*this, RawField(kCodeOffset), value);
 }
 
 void JSFunction::set_code_no_write_barrier(Code value) {
   DCHECK(!Heap::InNewSpace(value));
-  WRITE_FIELD(*this, kCodeOffset, value);
+  RELAXED_WRITE_FIELD(*this, kCodeOffset, value);
+}
+
+SharedFunctionInfo JSFunction::shared() const {
+  return SharedFunctionInfo::cast(
+      RELAXED_READ_FIELD(*this, kSharedFunctionInfoOffset));
+}
+
+void JSFunction::set_shared(SharedFunctionInfo value, WriteBarrierMode mode) {
+  // Release semantics to support acquire read in NeedsResetDueToFlushedBytecode
+  RELEASE_WRITE_FIELD(*this, kSharedFunctionInfoOffset, value);
+  CONDITIONAL_WRITE_BARRIER(*this, kSharedFunctionInfoOffset, value, mode);
 }
 
 void JSFunction::ClearOptimizedCodeSlot(const char* reason) {
@@ -659,17 +669,32 @@ bool JSFunction::is_compiled() const {
          shared()->is_compiled();
 }
 
+bool JSFunction::NeedsResetDueToFlushedBytecode() {
+  if (!FLAG_flush_bytecode) return false;
+
+  // Do a raw read for shared and code fields here since this function may be
+  // called on a concurrent thread and the JSFunction might not be fully
+  // initialized yet.
+  Object maybe_shared = ACQUIRE_READ_FIELD(*this, kSharedFunctionInfoOffset);
+  Object maybe_code = RELAXED_READ_FIELD(*this, kCodeOffset);
+
+  if (!maybe_shared->IsSharedFunctionInfo() || !maybe_code->IsCode()) {
+    return false;
+  }
+
+  SharedFunctionInfo shared = SharedFunctionInfo::cast(maybe_shared);
+  Code code = Code::cast(maybe_code);
+  return !shared->is_compiled() &&
+         code->builtin_index() != Builtins::kCompileLazy;
+}
+
 void JSFunction::ResetIfBytecodeFlushed() {
-  if (!shared()->is_compiled()) {
+  if (NeedsResetDueToFlushedBytecode()) {
     // Bytecode was flushed and function is now uncompiled, reset JSFunction
     // by setting code to CompileLazy and clearing the feedback vector.
-    if (code()->builtin_index() != Builtins::kCompileLazy) {
+    set_code(GetIsolate()->builtins()->builtin(i::Builtins::kCompileLazy));
-      set_code(GetIsolate()->builtins()->builtin(i::Builtins::kCompileLazy));
+    raw_feedback_cell()->set_value(
-    }
+        ReadOnlyRoots(GetIsolate()).undefined_value());
-    if (raw_feedback_cell()->value()->IsFeedbackVector()) {
-      raw_feedback_cell()->set_value(
-          ReadOnlyRoots(GetIsolate()).undefined_value());
-    }
   }
 }
 

src/objects/js-objects.h

@@ -1042,6 +1042,7 @@ class JSFunction : public JSObject {
   void ClearTypeFeedbackInfo();
 
   // Resets function to clear compiled data after bytecode has been flushed.
+  inline bool NeedsResetDueToFlushedBytecode();
   inline void ResetIfBytecodeFlushed();
 
   inline bool has_prototype_slot() const;
@@ -1097,8 +1098,6 @@ class JSFunction : public JSObject {
                                           int* instance_size,
                                           int* in_object_properties);
 
-  class BodyDescriptor;
-
   // Dispatched behavior.
   DECL_PRINTER(JSFunction)
   DECL_VERIFIER(JSFunction)

src/objects/map.h

@@ -43,6 +43,7 @@ enum InstanceType : uint16_t;
   V(JSApiObject)                       \
   V(JSArrayBuffer)                     \
   V(JSDataView)                        \
+  V(JSFunction)                        \
   V(JSObject)                          \
   V(JSObjectFast)                      \
   V(JSTypedArray)                      \