[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}

src/compiler/js-heap-broker.cc

@@ -516,33 +516,30 @@ ObjectData* JSObjectData::GetOwnDataProperty(JSHeapBroker* broker,
 class JSTypedArrayData : public JSObjectData {
  public:
   JSTypedArrayData(JSHeapBroker* broker, ObjectData** storage,
-                   Handle<JSTypedArray> object);
+                   Handle<JSTypedArray> object)
+      : JSObjectData(broker, storage, object) {}
 
-  bool is_on_heap() const { return is_on_heap_; }
-  size_t length() const { return length_; }
-  void* data_ptr() const { return data_ptr_; }
+  // 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_; }
 
+  bool is_on_heap() const { return is_on_heap_; }
+  size_t length() const { return length_; }
+  void* data_ptr() const { return data_ptr_; }
+
   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;
@@ -550,6 +547,10 @@ 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());
@@ -3455,10 +3456,6 @@ 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,
@@ -3690,8 +3687,50 @@ 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 {
-  if (data_->should_access_heap()) {
+  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);
     return object()->DataPtr();
   }
   return data()->AsJSTypedArray()->data_ptr();
@@ -4413,9 +4452,20 @@ void NativeContextRef::SerializeOnBackground() {
 }
 
 void JSTypedArrayRef::Serialize() {
-  if (data_->should_access_heap()) return;
-  CHECK_EQ(broker()->mode(), JSHeapBroker::kSerializing);
-  data()->AsJSTypedArray()->Serialize(broker());
+  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());
+  }
 }
 
 bool JSTypedArrayRef::serialized() const {

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

@@ -27,6 +27,10 @@ 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);
 }
@@ -253,15 +257,22 @@ 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()));
 }
 
@@ -274,10 +285,17 @@ 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;
-  // 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();
+  return base_pointer(tag) != Smi::zero();
 }
 
 // static

src/objects/js-array-buffer.h

@@ -254,7 +254,10 @@ class JSTypedArray
   static constexpr size_t kMaxLength = v8::TypedArray::kMaxLength;
 
   // [length]: length of typed array in elements.
-  DECL_PRIMITIVE_ACCESSORS(length, size_t)
+  DECL_PRIMITIVE_GETTER(length, size_t)
+
+  DECL_GETTER(base_pointer, Object)
+  DECL_ACQUIRE_GETTER(base_pointer, Object)
 
   // ES6 9.4.5.3
   V8_WARN_UNUSED_RESULT static Maybe<bool> DefineOwnProperty(
@@ -272,6 +275,10 @@ 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();
 
@@ -281,6 +288,7 @@ 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|
@@ -335,11 +343,16 @@ 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,15 +45,9 @@ 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,9 +48,13 @@
     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) \
-  inline type name() const;                  \
-  inline void set_##name(type value);
+  DECL_PRIMITIVE_GETTER(name, type)          \
+  DECL_PRIMITIVE_SETTER(name, type)
 
 #define DECL_SYNCHRONIZED_PRIMITIVE_ACCESSORS(name, type) \
   inline type synchronized_##name() const;                \