Revert "[compiler] Don't serialize JSTypedArray fields"

This reverts commit da785659.

Reason for revert: Investigating regressions https://chromeperf.appspot.com/group_report?rev=72572

Original change's description:
> [compiler] Don't serialize JSTypedArray fields
>
> This CL removes serialization of JSTypedArray fields when direct heap
> reads are enabled. Invariants we rely on:
>
> - Of the underlying interesting fields,
>   - base_pointer and external_pointer are set either during
>     initialization, or in a one-time on-to-off-heap transition in
>     GetBuffer.
>   - length and buffer are immutable after initialization.
> - is_on_heap and DataPtr derive from base_pointer and
>   external_pointer s.t. is_on_heap == (base_pointer != 0) and
>   DataPtr == external_pointer in the off-heap case.
>
> In this CL we add one new invariant:
>
> - For all base_pointer and external_pointer mutations after
>   initialization, base_pointer is guaranteed to be release-stored
>   after external_pointer has been written.
>
> With these invariants, concurrent access to off-heap typed arrays is
> trivial as long as is_on_heap (= base_pointer) is read before other
> relevant fields.
>
> Note that JSTypedArray remains a kSerializedHeapObject due to the
> serialized superclass JSObject.
>
> Drive-by: Remove unused Torque operators and empty TODOs.
>
> Bug: v8:7790
> Change-Id: I3c4327318f94e4e6083d4e87476069aad2649386
> Cq-Include-Trybots: luci.v8.try:v8_linux64_tsan_isolates_rel_ng
> Cq-Include-Trybots: luci.v8.try:v8_linux64_tsan_rel_ng
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2679689
> Commit-Queue: Jakob Gruber <jgruber@chromium.org>
> Reviewed-by: Georg Neis <neis@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#72572}

TBR=neis@chromium.org,jgruber@chromium.org

Change-Id: I5a7e6bacb7b7a3e3510c778837679e6822f26339
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:7790
Cq-Include-Trybots: luci.v8.try:v8_linux64_tsan_isolates_rel_ng
Cq-Include-Trybots: luci.v8.try:v8_linux64_tsan_rel_ng
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2681948Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#72583}

src/compiler/js-heap-broker.cc

@@ -516,30 +516,33 @@ ObjectData* JSObjectData::GetOwnDataProperty(JSHeapBroker* broker,
 class JSTypedArrayData : public JSObjectData {
  public:
   JSTypedArrayData(JSHeapBroker* broker, ObjectData** storage,
-                   Handle<JSTypedArray> object)
-      : JSObjectData(broker, storage, object) {}
-
-  // TODO(v8:7790): Once JSObject is no longer serialized, also make
-  // JSTypedArrayRef never-serialized.
-  STATIC_ASSERT(IsSerializedHeapObject<JSObject>());
-
-  void Serialize(JSHeapBroker* broker);
-  bool serialized() const { return serialized_; }
+                   Handle<JSTypedArray> object);
 
   bool is_on_heap() const { return is_on_heap_; }
   size_t length() const { return length_; }
   void* data_ptr() const { return data_ptr_; }
 
+  void Serialize(JSHeapBroker* broker);
+  bool serialized() const { return serialized_; }
+
   ObjectData* buffer() const { return buffer_; }
 
  private:
+  bool const is_on_heap_;
+  size_t const length_;
+  void* const data_ptr_;
+
   bool serialized_ = false;
-  bool is_on_heap_ = false;
-  size_t length_ = 0;
-  void* data_ptr_ = nullptr;
   ObjectData* buffer_ = nullptr;
 };
 
+JSTypedArrayData::JSTypedArrayData(JSHeapBroker* broker, ObjectData** storage,
+                                   Handle<JSTypedArray> object)
+    : JSObjectData(broker, storage, object),
+      is_on_heap_(object->is_on_heap()),
+      length_(object->length()),
+      data_ptr_(object->DataPtr()) {}
+
 void JSTypedArrayData::Serialize(JSHeapBroker* broker) {
   if (serialized_) return;
   serialized_ = true;
@@ -547,10 +550,6 @@ void JSTypedArrayData::Serialize(JSHeapBroker* broker) {
   TraceScope tracer(broker, this, "JSTypedArrayData::Serialize");
   Handle<JSTypedArray> typed_array = Handle<JSTypedArray>::cast(object());
 
-  is_on_heap_ = typed_array->is_on_heap();
-  length_ = typed_array->length();
-  data_ptr_ = typed_array->DataPtr();
-
   if (!is_on_heap()) {
     DCHECK_NULL(buffer_);
     buffer_ = broker->GetOrCreateData(typed_array->buffer());
@@ -3456,6 +3455,10 @@ BIMODAL_ACCESSOR(JSFunction, Code, code)
 
 BIMODAL_ACCESSOR_C(JSGlobalObject, bool, IsDetached)
 
+BIMODAL_ACCESSOR_C(JSTypedArray, bool, is_on_heap)
+BIMODAL_ACCESSOR_C(JSTypedArray, size_t, length)
+BIMODAL_ACCESSOR(JSTypedArray, HeapObject, buffer)
+
 BIMODAL_ACCESSOR_WITH_FLAG_B(Map, bit_field2, elements_kind,
                              Map::Bits2::ElementsKindBits)
 BIMODAL_ACCESSOR_WITH_FLAG_B(Map, bit_field3, is_dictionary_map,
@@ -3687,50 +3690,8 @@ base::Optional<MapRef> MapRef::FindRootMap() const {
   return base::nullopt;
 }
 
-bool JSTypedArrayRef::is_on_heap() const {
-  if (data_->should_access_heap() || FLAG_turbo_direct_heap_access) {
-    // Safe to read concurrently because:
-    // - host object seen by serializer.
-    // - underlying field written 1. during initialization or 2. with
-    //   release-store.
-    return object()->is_on_heap(kAcquireLoad);
-  }
-  return data()->AsJSTypedArray()->data_ptr();
-}
-
-size_t JSTypedArrayRef::length() const {
-  CHECK(!is_on_heap());
-  if (data_->should_access_heap() || FLAG_turbo_direct_heap_access) {
-    // Safe to read concurrently because:
-    // - immutable after initialization.
-    // - host object seen by serializer.
-    return object()->length();
-  }
-  return data()->AsJSTypedArray()->length();
-}
-
-HeapObjectRef JSTypedArrayRef::buffer() const {
-  CHECK(!is_on_heap());
-  if (data_->should_access_heap() || FLAG_turbo_direct_heap_access) {
-    // Safe to read concurrently because:
-    // - immutable after initialization.
-    // - host object seen by serializer.
-    Handle<JSArrayBuffer> value(object()->buffer(), broker()->isolate());
-    return JSObjectRef{broker(), value};
-  }
-  return HeapObjectRef{broker(), data()->AsJSTypedArray()->buffer()};
-}
-
 void* JSTypedArrayRef::data_ptr() const {
-  CHECK(!is_on_heap());
-  if (data_->should_access_heap() || FLAG_turbo_direct_heap_access) {
-    // Safe to read concurrently because:
-    // - host object seen by serializer.
-    // - underlying field written 1. during initialization or 2. protected by
-    //   the is_on_heap release/acquire semantics (external_pointer store
-    //   happens-before base_pointer store, and this external_pointer load
-    //   happens-after base_pointer load).
-    STATIC_ASSERT(JSTypedArray::kOffHeapDataPtrEqualsExternalPointer);
+  if (data_->should_access_heap()) {
     return object()->DataPtr();
   }
   return data()->AsJSTypedArray()->data_ptr();
@@ -4452,20 +4413,9 @@ void NativeContextRef::SerializeOnBackground() {
 }
 
 void JSTypedArrayRef::Serialize() {
-  if (data_->should_access_heap() || FLAG_turbo_direct_heap_access) {
-    // Even if the typed array object itself is no longer serialized (besides
-    // the JSObject parts), the `buffer` field still is and thus we need to
-    // make sure to visit it.
-    // TODO(jgruber,v8:7790): Remove once JSObject is no longer serialized.
-    static_assert(
-        std::is_base_of<JSObject, decltype(object()->buffer())>::value, "");
-    STATIC_ASSERT(IsSerializedHeapObject<JSObject>());
-    JSObjectRef data_ref{
-        broker(), broker()->CanonicalPersistentHandle(object()->buffer())};
-  } else {
-    CHECK_EQ(broker()->mode(), JSHeapBroker::kSerializing);
-    data()->AsJSTypedArray()->Serialize(broker());
-  }
+  if (data_->should_access_heap()) return;
+  CHECK_EQ(broker()->mode(), JSHeapBroker::kSerializing);
+  data()->AsJSTypedArray()->Serialize(broker());
 }
 
 bool JSTypedArrayRef::serialized() const {

src/objects/js-array-buffer-inl.h

@@ -27,10 +27,6 @@ TQ_OBJECT_CONSTRUCTORS_IMPL(JSArrayBufferView)
 TQ_OBJECT_CONSTRUCTORS_IMPL(JSTypedArray)
 TQ_OBJECT_CONSTRUCTORS_IMPL(JSDataView)
 
-ACCESSORS(JSTypedArray, base_pointer, Object, kBasePointerOffset)
-RELEASE_ACQUIRE_ACCESSORS(JSTypedArray, base_pointer, Object,
-                          kBasePointerOffset)
-
 void JSArrayBuffer::AllocateExternalPointerEntries(Isolate* isolate) {
   InitExternalPointerField(kBackingStoreOffset, isolate);
 }
@@ -257,22 +253,15 @@ void* JSTypedArray::DataPtr() {
   // so that the addition with |external_pointer| (which already contains
   // compensated offset value) will decompress the tagged value.
   // See JSTypedArray::ExternalPointerCompensationForOnHeapArray() for details.
-  STATIC_ASSERT(kOffHeapDataPtrEqualsExternalPointer);
   return reinterpret_cast<void*>(external_pointer() +
                                  static_cast<Tagged_t>(base_pointer().ptr()));
 }
 
 void JSTypedArray::SetOffHeapDataPtr(Isolate* isolate, void* base,
                                      Address offset) {
+  set_base_pointer(Smi::zero(), SKIP_WRITE_BARRIER);
   Address address = reinterpret_cast<Address>(base) + offset;
   set_external_pointer(isolate, address);
-  // This is the only spot in which the `base_pointer` field can be mutated
-  // after object initialization. Note this can happen at most once, when
-  // `JSTypedArray::GetBuffer` transitions from an on- to off-heap
-  // representation.
-  // To play well with Turbofan concurrency requirements, `base_pointer` is set
-  // with a release store, after external_pointer has been set.
-  set_base_pointer(Smi::zero(), kReleaseStore, SKIP_WRITE_BARRIER);
   DCHECK_EQ(address, reinterpret_cast<Address>(DataPtr()));
 }
 
@@ -285,17 +274,10 @@ void JSTypedArray::SetOnHeapDataPtr(Isolate* isolate, HeapObject base,
 }
 
 bool JSTypedArray::is_on_heap() const {
-  // Keep synced with `is_on_heap(AcquireLoadTag)`.
-  DisallowGarbageCollection no_gc;
-  return base_pointer() != Smi::zero();
-}
-
-bool JSTypedArray::is_on_heap(AcquireLoadTag tag) const {
-  // Keep synced with `is_on_heap()`.
-  // Note: For Turbofan concurrency requirements, it's important that this
-  // function reads only `base_pointer`.
   DisallowGarbageCollection no_gc;
-  return base_pointer(tag) != Smi::zero();
+  // Checking that buffer()->backing_store() is not nullptr is not sufficient;
+  // it will be nullptr when byte_length is 0 as well.
+  return base_pointer() == elements();
 }
 
 // static

src/objects/js-array-buffer.h

@@ -254,10 +254,7 @@ class JSTypedArray
   static constexpr size_t kMaxLength = v8::TypedArray::kMaxLength;
 
   // [length]: length of typed array in elements.
-  DECL_PRIMITIVE_GETTER(length, size_t)
-
-  DECL_GETTER(base_pointer, Object)
-  DECL_ACQUIRE_GETTER(base_pointer, Object)
+  DECL_PRIMITIVE_ACCESSORS(length, size_t)
 
   // ES6 9.4.5.3
   V8_WARN_UNUSED_RESULT static Maybe<bool> DefineOwnProperty(
@@ -275,10 +272,6 @@ class JSTypedArray
   // When sandbox is not enabled, it's a no-op.
   inline void AllocateExternalPointerEntries(Isolate* isolate);
 
-  // The `DataPtr` is `base_ptr + external_pointer`, and `base_ptr` is nullptr
-  // for off-heap typed arrays.
-  static constexpr bool kOffHeapDataPtrEqualsExternalPointer = true;
-
   // Use with care: returns raw pointer into heap.
   inline void* DataPtr();
 
@@ -288,7 +281,6 @@ class JSTypedArray
 
   // Whether the buffer's backing store is on-heap or off-heap.
   inline bool is_on_heap() const;
-  inline bool is_on_heap(AcquireLoadTag tag) const;
 
   // Note: this is a pointer compression specific optimization.
   // Normally, on-heap typed arrays contain HeapObject value in |base_pointer|
@@ -343,16 +335,11 @@ class JSTypedArray
 
  private:
   friend class Deserializer;
-  friend class Factory;
-
-  DECL_PRIMITIVE_SETTER(length, size_t)
 
+  // [external_pointer]: TODO(v8:4153)
   DECL_GETTER(external_pointer, Address)
   DECL_GETTER(external_pointer_raw, ExternalPointer_t)
 
-  DECL_SETTER(base_pointer, Object)
-  DECL_RELEASE_SETTER(base_pointer, Object)
-
   inline void set_external_pointer(Isolate* isolate, Address value);
 
   TQ_OBJECT_CONSTRUCTORS(JSTypedArray)

src/objects/js-array-buffer.tq

@@ -45,9 +45,15 @@ extern class JSArrayBufferView extends JSObject {
 extern class JSTypedArray extends JSArrayBufferView {
   length: uintptr;
   external_pointer: ExternalPointer;
+  // [base_pointer]: TODO(v8:4153)
   base_pointer: ByteArray|Smi;
 }
 
+extern operator '.external_pointer_ptr' macro
+LoadJSTypedArrayExternalPointerPtr(JSTypedArray): RawPtr;
+extern operator '.external_pointer_ptr=' macro
+StoreJSTypedArrayExternalPointerPtr(JSTypedArray, RawPtr);
+
 @generateCppClass
 extern class JSDataView extends JSArrayBufferView {
   data_pointer: ExternalPointer;

src/objects/object-macros.h

@@ -48,13 +48,9 @@
     return GetIsolateFromWritableObject(*this);                            \
   }
 
-#define DECL_PRIMITIVE_GETTER(name, type) inline type name() const;
-
-#define DECL_PRIMITIVE_SETTER(name, type) inline void set_##name(type value);
-
 #define DECL_PRIMITIVE_ACCESSORS(name, type) \
-  DECL_PRIMITIVE_GETTER(name, type)          \
-  DECL_PRIMITIVE_SETTER(name, type)
+  inline type name() const;                  \
+  inline void set_##name(type value);
 
 #define DECL_SYNCHRONIZED_PRIMITIVE_ACCESSORS(name, type) \
   inline type synchronized_##name() const;                \