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

This reverts commit f5729f1cda.

Reason for revert: GC Stress failures, e.g. https://ci.chromium.org/p/v8/builders/luci.v8.ci/V8%20Linux64%20GC%20Stress%20-%20custom%20snapshot/23549

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}

TBR=ulan@chromium.org,rmcilroy@chromium.org

Change-Id: I1ba0a190e54bb84b9e2c52ae73d19eb5afc02a4b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:8395
Reviewed-on: https://chromium-review.googlesource.com/c/1408993
Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58778}

src/heap-symbols.h

@@ -383,7 +383,6 @@
   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,17 +387,6 @@ 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);
@@ -836,7 +825,6 @@ 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,19 +93,6 @@ 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,
@@ -499,10 +486,6 @@ 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

@@ -1906,11 +1906,6 @@ 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.
@@ -1940,7 +1935,6 @@ 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() {
@@ -2079,15 +2073,6 @@ 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)) {
@@ -2345,7 +2330,6 @@ 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,7 +449,6 @@ struct WeakObjects {
   Worklist<JSWeakCell, 64> js_weak_cells;
 
   Worklist<SharedFunctionInfo, 64> bytecode_flushing_candidates;
-  Worklist<JSFunction, 64> flushed_js_functions;
 };
 
 struct EphemeronMarking {
@@ -663,7 +662,6 @@ 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;
@@ -795,9 +793,6 @@ 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();
@@ -932,7 +927,6 @@ 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,7 +55,6 @@ 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,6 +197,30 @@ 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

@@ -3214,9 +3214,6 @@ 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;
 
@@ -3257,6 +3254,7 @@ 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,6 +459,7 @@ 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)
@@ -541,29 +542,18 @@ AbstractCode JSFunction::abstract_code() {
 }
 
 Code JSFunction::code() const {
-  return Code::cast(RELAXED_READ_FIELD(*this, kCodeOffset));
+  return Code::cast(READ_FIELD(*this, kCodeOffset));
 }
 
 void JSFunction::set_code(Code value) {
   DCHECK(!Heap::InNewSpace(value));
-  RELAXED_WRITE_FIELD(*this, kCodeOffset, value);
+  WRITE_FIELD(*this, kCodeOffset, value);
   MarkingBarrier(*this, RawField(kCodeOffset), value);
 }
 
 void JSFunction::set_code_no_write_barrier(Code value) {
   DCHECK(!Heap::InNewSpace(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);
+  WRITE_FIELD(*this, kCodeOffset, value);
 }
 
 void JSFunction::ClearOptimizedCodeSlot(const char* reason) {
@@ -669,32 +659,17 @@ 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 (NeedsResetDueToFlushedBytecode()) {
+  if (!shared()->is_compiled()) {
     // Bytecode was flushed and function is now uncompiled, reset JSFunction
     // by setting code to CompileLazy and clearing the feedback vector.
-    set_code(GetIsolate()->builtins()->builtin(i::Builtins::kCompileLazy));
-    raw_feedback_cell()->set_value(
-        ReadOnlyRoots(GetIsolate()).undefined_value());
+    if (code()->builtin_index() != Builtins::kCompileLazy) {
+      set_code(GetIsolate()->builtins()->builtin(i::Builtins::kCompileLazy));
+    }
+    if (raw_feedback_cell()->value()->IsFeedbackVector()) {
+      raw_feedback_cell()->set_value(
+          ReadOnlyRoots(GetIsolate()).undefined_value());
+    }
   }
 }
 

src/objects/js-objects.h

@@ -1042,7 +1042,6 @@ 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;
@@ -1098,6 +1097,8 @@ 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,7 +43,6 @@ enum InstanceType : uint16_t;
   V(JSApiObject)                       \
   V(JSArrayBuffer)                     \
   V(JSDataView)                        \
-  V(JSFunction)                        \
   V(JSObject)                          \
   V(JSObjectFast)                      \
   V(JSTypedArray)                      \