Reland: ScopeInfo, SharedFunctionInfo never-ever serialized

This relands squashed CLs:

59b9aaf7
8f84d0bb

The revert was at crrev.com/c/2996198.

Changed: Fixed a test in which bytecode flushing caused a behavioral
change between serialized- and unserialized SFI Refs. The serialized
SFI ref kept bytecode alive while unserialized SFIs allow flushing.
The test was fixed by adding a %PrepareFunctionForOptimization
annotation.

No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:7790, v8:11939
Change-Id: I170f8085bd7454a2a5f2bb03c8824e2862857827
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2999089
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Auto-Submit: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#75504}

src/compiler/heap-refs.cc

@@ -1896,129 +1896,25 @@ ObjectData* JSArrayData::GetOwnElement(JSHeapBroker* broker, uint32_t index,
 class ScopeInfoData : public HeapObjectData {
  public:
   ScopeInfoData(JSHeapBroker* broker, ObjectData** storage,
-                Handle<ScopeInfo> object);
-
-  int ContextLength() const { return context_length_; }
-  bool HasContextExtensionSlot() const { return has_context_extension_slot_; }
-  bool HasOuterScopeInfo() const { return has_outer_scope_info_; }
-
-  ObjectData* OuterScopeInfo() const { return outer_scope_info_; }
-  void SerializeScopeInfoChain(JSHeapBroker* broker);
-
- private:
-  int const context_length_;
-  bool const has_context_extension_slot_;
-  bool const has_outer_scope_info_;
-
-  // Only serialized via SerializeScopeInfoChain.
-  ObjectData* outer_scope_info_;
-};
-
-ScopeInfoData::ScopeInfoData(JSHeapBroker* broker, ObjectData** storage,
-                             Handle<ScopeInfo> object)
-    : HeapObjectData(broker, storage, object),
-      context_length_(object->ContextLength()),
-      has_context_extension_slot_(object->HasContextExtensionSlot()),
-      has_outer_scope_info_(object->HasOuterScopeInfo()),
-      outer_scope_info_(nullptr) {
-  DCHECK(!broker->is_concurrent_inlining());
-}
-
-void ScopeInfoData::SerializeScopeInfoChain(JSHeapBroker* broker) {
-  if (outer_scope_info_) return;
-  if (!has_outer_scope_info_) return;
-  outer_scope_info_ = broker->GetOrCreateData(
-      Handle<ScopeInfo>::cast(object())->OuterScopeInfo());
-  if (!outer_scope_info_->should_access_heap()) {
-    outer_scope_info_->AsScopeInfo()->SerializeScopeInfoChain(broker);
+                Handle<ScopeInfo> object)
+      : HeapObjectData(broker, storage, object) {
+    // TODO(v8:7790): Remove this class once all kNeverSerialized types are
+    // NeverEverSerialize.
+    UNREACHABLE();
   }
-}
+};
 
 class SharedFunctionInfoData : public HeapObjectData {
  public:
   SharedFunctionInfoData(JSHeapBroker* broker, ObjectData** storage,
-                         Handle<SharedFunctionInfo> object);
-
-  Builtin builtin_id() const { return builtin_id_; }
-  int context_header_size() const { return context_header_size_; }
-  ObjectData* GetBytecodeArray() const { return GetBytecodeArray_; }
-  SharedFunctionInfo::Inlineability GetInlineability() const {
-    return inlineability_;
-  }
-  void SerializeFunctionTemplateInfo(JSHeapBroker* broker);
-  ObjectData* scope_info() const { return scope_info_; }
-  void SerializeScopeInfoChain(JSHeapBroker* broker);
-  ObjectData* function_template_info() const { return function_template_info_; }
-  ObjectData* GetTemplateObject(FeedbackSlot slot) const {
-    auto lookup_it = template_objects_.find(slot.ToInt());
-    if (lookup_it != template_objects_.cend()) {
-      return lookup_it->second;
-    }
-    return nullptr;
-  }
-  void SetTemplateObject(FeedbackSlot slot, ObjectData* object) {
-    CHECK(
-        template_objects_.insert(std::make_pair(slot.ToInt(), object)).second);
+                         Handle<SharedFunctionInfo> object)
+      : HeapObjectData(broker, storage, object) {
+    // TODO(v8:7790): Remove this class once all kNeverSerialized types are
+    // NeverEverSerialize.
+    UNREACHABLE();
   }
-
-#define DECL_ACCESSOR(type, name) \
-  type name() const { return name##_; }
-  BROKER_SFI_FIELDS(DECL_ACCESSOR)
-#undef DECL_ACCESSOR
-
- private:
-  Builtin const builtin_id_;
-  int const context_header_size_;
-  ObjectData* const GetBytecodeArray_;
-#define DECL_MEMBER(type, name) type const name##_;
-  BROKER_SFI_FIELDS(DECL_MEMBER)
-#undef DECL_MEMBER
-  SharedFunctionInfo::Inlineability const inlineability_;
-  ObjectData* function_template_info_;
-  ZoneMap<int, ObjectData*> template_objects_;
-  ObjectData* scope_info_;
 };
 
-SharedFunctionInfoData::SharedFunctionInfoData(
-    JSHeapBroker* broker, ObjectData** storage,
-    Handle<SharedFunctionInfo> object)
-    : HeapObjectData(broker, storage, object),
-      builtin_id_(object->HasBuiltinId() ? object->builtin_id()
-                                         : Builtin::kNoBuiltinId),
-      context_header_size_(object->scope_info().ContextHeaderLength()),
-      GetBytecodeArray_(object->HasBytecodeArray()
-                            ? broker->GetOrCreateData(
-                                  object->GetBytecodeArray(broker->isolate()))
-                            : nullptr)
-#define INIT_MEMBER(type, name) , name##_(object->name())
-          BROKER_SFI_FIELDS(INIT_MEMBER)
-#undef INIT_MEMBER
-      ,
-      inlineability_(
-          object->GetInlineability(broker->isolate(), broker->is_turboprop())),
-      function_template_info_(nullptr),
-      template_objects_(broker->zone()),
-      scope_info_(nullptr) {
-  DCHECK_EQ(HasBuiltinId_, builtin_id_ != Builtin::kNoBuiltinId);
-  DCHECK_EQ(HasBytecodeArray_, GetBytecodeArray_ != nullptr);
-}
-
-void SharedFunctionInfoData::SerializeFunctionTemplateInfo(
-    JSHeapBroker* broker) {
-  if (function_template_info_) return;
-  function_template_info_ = broker->GetOrCreateData(
-      Handle<SharedFunctionInfo>::cast(object())->function_data(kAcquireLoad));
-}
-
-void SharedFunctionInfoData::SerializeScopeInfoChain(JSHeapBroker* broker) {
-  if (scope_info_) return;
-  scope_info_ = broker->GetOrCreateData(
-      Handle<SharedFunctionInfo>::cast(object())->scope_info());
-  if (!scope_info_->should_access_heap()) {
-    scope_info_->AsScopeInfo()->SerializeScopeInfoChain(broker);
-  }
-}
-
 class SourceTextModuleData : public HeapObjectData {
  public:
   SourceTextModuleData(JSHeapBroker* broker, ObjectData** storage,
@@ -2729,6 +2625,8 @@ NEVER_EVER_SERIALIZE(Context)
 NEVER_EVER_SERIALIZE(NativeContext)
 NEVER_EVER_SERIALIZE(ObjectBoilerplateDescription)
 NEVER_EVER_SERIALIZE(RegExpBoilerplateDescription)
+NEVER_EVER_SERIALIZE(SharedFunctionInfo)
+NEVER_EVER_SERIALIZE(ScopeInfo)
 NEVER_EVER_SERIALIZE(TemplateObjectDescription)
 
 #undef NEVER_EVER_SERIALIZE
@@ -3256,6 +3154,14 @@ int BytecodeArrayRef::handler_table_size() const {
     return BitField::decode(ObjectRef::data()->As##holder()->field()); \
   }
 
+#define HEAP_ACCESSOR(holder, result, name)                   \
+  result##Ref holder##Ref::name() const {                     \
+    return MakeRef(broker(), result::cast(object()->name())); \
+  }
+
+#define HEAP_ACCESSOR_C(holder, result, name) \
+  result holder##Ref::name() const { return object()->name(); }
+
 BIMODAL_ACCESSOR(AllocationSite, Object, nested_site)
 BIMODAL_ACCESSOR_C(AllocationSite, bool, CanInlineCall)
 BIMODAL_ACCESSOR_C(AllocationSite, bool, PointsToLiteral)
@@ -3471,41 +3377,35 @@ HolderLookupResult FunctionTemplateInfoRef::LookupHolderOfExpectedType(
 
 BIMODAL_ACCESSOR(CallHandlerInfo, Object, data)
 
-BIMODAL_ACCESSOR_C(ScopeInfo, int, ContextLength)
-BIMODAL_ACCESSOR_C(ScopeInfo, bool, HasContextExtensionSlot)
-BIMODAL_ACCESSOR_C(ScopeInfo, bool, HasOuterScopeInfo)
-BIMODAL_ACCESSOR(ScopeInfo, ScopeInfo, OuterScopeInfo)
+HEAP_ACCESSOR_C(ScopeInfo, int, ContextLength)
+HEAP_ACCESSOR_C(ScopeInfo, bool, HasContextExtensionSlot)
+HEAP_ACCESSOR_C(ScopeInfo, bool, HasOuterScopeInfo)
+HEAP_ACCESSOR(ScopeInfo, ScopeInfo, OuterScopeInfo)
+
+HEAP_ACCESSOR_C(SharedFunctionInfo, Builtin, builtin_id)
 
-BIMODAL_ACCESSOR_C(SharedFunctionInfo, Builtin, builtin_id)
 BytecodeArrayRef SharedFunctionInfoRef::GetBytecodeArray() const {
-  if (data_->should_access_heap() || broker()->is_concurrent_inlining()) {
-    BytecodeArray bytecode_array;
-    if (!broker()->IsMainThread()) {
-      bytecode_array = object()->GetBytecodeArray(broker()->local_isolate());
-    } else {
-      bytecode_array = object()->GetBytecodeArray(broker()->isolate());
-    }
-    return MakeRef(broker(), bytecode_array);
+  BytecodeArray bytecode_array;
+  if (!broker()->IsMainThread()) {
+    bytecode_array = object()->GetBytecodeArray(broker()->local_isolate());
+  } else {
+    bytecode_array = object()->GetBytecodeArray(broker()->isolate());
   }
-  return BytecodeArrayRef(
-      broker(), ObjectRef::data()->AsSharedFunctionInfo()->GetBytecodeArray());
+  return MakeRefAssumeMemoryFence(broker(), bytecode_array);
 }
+
 #define DEF_SFI_ACCESSOR(type, name) \
-  BIMODAL_ACCESSOR_WITH_FLAG_C(SharedFunctionInfo, type, name)
+  HEAP_ACCESSOR_C(SharedFunctionInfo, type, name)
 BROKER_SFI_FIELDS(DEF_SFI_ACCESSOR)
 #undef DEF_SFI_ACCESSOR
+
 SharedFunctionInfo::Inlineability SharedFunctionInfoRef::GetInlineability()
     const {
-  if (data_->should_access_heap()) {
-    if (!broker()->IsMainThread()) {
-      return object()->GetInlineability(broker()->local_isolate(),
-                                        broker()->is_turboprop());
-    } else {
-      return object()->GetInlineability(broker()->isolate(),
-                                        broker()->is_turboprop());
-    }
-  }
-  return ObjectRef::data()->AsSharedFunctionInfo()->GetInlineability();
+  return broker()->IsMainThread()
+             ? object()->GetInlineability(broker()->isolate(),
+                                          broker()->is_turboprop())
+             : object()->GetInlineability(broker()->local_isolate(),
+                                          broker()->is_turboprop());
 }
 
 base::Optional<FeedbackVectorRef> FeedbackCellRef::value() const {
@@ -3657,12 +3557,6 @@ int MapRef::GetInObjectProperties() const {
   return data()->AsMap()->in_object_properties();
 }
 
-void ScopeInfoRef::SerializeScopeInfoChain() {
-  if (data_->should_access_heap()) return;
-  CHECK_EQ(broker()->mode(), JSHeapBroker::kSerializing);
-  data()->AsScopeInfo()->SerializeScopeInfoChain(broker());
-}
-
 bool StringRef::IsExternalString() const {
   IF_ACCESS_FROM_HEAP_C(IsExternalString);
   return data()->AsString()->is_external_string();
@@ -4297,41 +4191,19 @@ CodeRef JSFunctionRef::code() const {
   return CodeRef(broker(), ObjectRef::data()->AsJSFunction()->code());
 }
 
-void SharedFunctionInfoRef::SerializeFunctionTemplateInfo() {
-  if (data_->should_access_heap()) return;
-  CHECK_EQ(broker()->mode(), JSHeapBroker::kSerializing);
-  data()->AsSharedFunctionInfo()->SerializeFunctionTemplateInfo(broker());
-}
-
-void SharedFunctionInfoRef::SerializeScopeInfoChain() {
-  if (data_->should_access_heap()) return;
-  CHECK_EQ(broker()->mode(), JSHeapBroker::kSerializing);
-  data()->AsSharedFunctionInfo()->SerializeScopeInfoChain(broker());
-}
-
 base::Optional<FunctionTemplateInfoRef>
 SharedFunctionInfoRef::function_template_info() const {
-  if (data_->should_access_heap()) {
-    if (!object()->IsApiFunction()) return {};
-    return TryMakeRef(broker(), FunctionTemplateInfo::cast(
-                                    object()->function_data(kAcquireLoad)));
-  }
-  ObjectData* function_template_info =
-      data()->AsSharedFunctionInfo()->function_template_info();
-  if (!function_template_info) return base::nullopt;
-  return FunctionTemplateInfoRef(broker(), function_template_info);
+  if (!object()->IsApiFunction()) return {};
+  return TryMakeRef(broker(), FunctionTemplateInfo::cast(
+                                  object()->function_data(kAcquireLoad)));
 }
 
 int SharedFunctionInfoRef::context_header_size() const {
-  IF_ACCESS_FROM_HEAP_C(scope_info().ContextHeaderLength);
-  return data()->AsSharedFunctionInfo()->context_header_size();
+  return object()->scope_info().ContextHeaderLength();
 }
 
 ScopeInfoRef SharedFunctionInfoRef::scope_info() const {
-  if (data_->should_access_heap()) {
-    return MakeRef(broker(), object()->scope_info());
-  }
-  return ScopeInfoRef(broker(), data()->AsSharedFunctionInfo()->scope_info());
+  return MakeRef(broker(), object()->scope_info());
 }
 
 void JSObjectRef::SerializeObjectCreateMap() {
@@ -4508,8 +4380,15 @@ unsigned CodeRef::GetInlinedBytecodeSize() const {
 #undef BIMODAL_ACCESSOR
 #undef BIMODAL_ACCESSOR_B
 #undef BIMODAL_ACCESSOR_C
+#undef BIMODAL_ACCESSOR_WITH_FLAG
+#undef BIMODAL_ACCESSOR_WITH_FLAG_B
+#undef BIMODAL_ACCESSOR_WITH_FLAG_C
+#undef HEAP_ACCESSOR
+#undef HEAP_ACCESSOR_C
 #undef IF_ACCESS_FROM_HEAP
 #undef IF_ACCESS_FROM_HEAP_C
+#undef IF_ACCESS_FROM_HEAP_WITH_FLAG
+#undef IF_ACCESS_FROM_HEAP_WITH_FLAG_C
 #undef TRACE
 #undef TRACE_MISSING
 

src/compiler/heap-refs.h

@@ -899,6 +899,8 @@ class V8_EXPORT_PRIVATE SharedFunctionInfoRef : public HeapObjectRef {
   int context_header_size() const;
   BytecodeArrayRef GetBytecodeArray() const;
   SharedFunctionInfo::Inlineability GetInlineability() const;
+  base::Optional<FunctionTemplateInfoRef> function_template_info() const;
+  ScopeInfoRef scope_info() const;
 
 #define DECL_ACCESSOR(type, name) type name() const;
   BROKER_SFI_FIELDS(DECL_ACCESSOR)
@@ -907,12 +909,6 @@ class V8_EXPORT_PRIVATE SharedFunctionInfoRef : public HeapObjectRef {
   bool IsInlineable() const {
     return GetInlineability() == SharedFunctionInfo::kIsInlineable;
   }
-
-  void SerializeFunctionTemplateInfo();
-  base::Optional<FunctionTemplateInfoRef> function_template_info() const;
-
-  void SerializeScopeInfoChain();
-  ScopeInfoRef scope_info() const;
 };
 
 class StringRef : public NameRef {

src/compiler/serializer-for-background-compilation.cc

@@ -1783,12 +1783,6 @@ void SerializerForBackgroundCompilation::VisitForInPrepare(
 
 void SerializerForBackgroundCompilation::ProcessCreateContext(
     interpreter::BytecodeArrayIterator* iterator, int scopeinfo_operand_index) {
-  Handle<ScopeInfo> scope_info =
-      Handle<ScopeInfo>::cast(iterator->GetConstantForIndexOperand(
-          scopeinfo_operand_index, broker()->isolate()));
-  ScopeInfoRef scope_info_ref = MakeRef(broker(), scope_info);
-  scope_info_ref.SerializeScopeInfoChain();
-
   Hints const& current_context_hints = environment()->current_context_hints();
   Hints result_hints;
 
@@ -2245,9 +2239,6 @@ void SerializerForBackgroundCompilation::ProcessApiCall(
   if (!target_template_info.has_call_code()) return;
   target_template_info.SerializeCallCode();
 
-  SharedFunctionInfoRef target_ref = MakeRef(broker(), target);
-  target_ref.SerializeFunctionTemplateInfo();
-
   if (target_template_info.accept_any_receiver() &&
       target_template_info.is_signature_undefined()) {
     return;
@@ -2893,8 +2884,6 @@ void SerializerForBackgroundCompilation::ProcessCheckContextExtensions(
     ProcessContextAccess(context_hints, Context::EXTENSION_INDEX, i,
                          kSerializeSlot);
   }
-  SharedFunctionInfoRef shared = MakeRef(broker(), function().shared());
-  shared.SerializeScopeInfoChain();
 }
 
 void SerializerForBackgroundCompilation::ProcessLdaLookupGlobalSlot(

test/mjsunit/es6/map-constructor-entry-side-effect2.js

@@ -9,16 +9,18 @@ function TestMapConstructorEntrySideEffect(ctor) {
   const k1 = {};
   const k2 = {};
   let callCount = 0;
+  let prop = {
+    get() {
+      // Verify continuation retains original set function
+      ctor.prototype.set = () => {
+        callCount++;
+      };
+      return k1;
+    }
+  };
+  %PrepareFunctionForOptimization(prop.get);  // To prevent flushing.
   const input = [
-    Object.defineProperty([, 1], "0", {
-      get() {
-        // Verify continuation retains original set function
-        ctor.prototype.set = () => {
-          callCount++;
-        };
-        return k1;
-      }
-    }),
+    Object.defineProperty([, 1], "0", prop),
     [k2, 2]
   ];
   const col = new ctor(input);