From fe8f3e6fd6e57b1c690e9e4afb983d2a1ff1d2c9 Mon Sep 17 00:00:00 2001 From: Jakob Gruber Date: Wed, 30 Jun 2021 07:42:02 +0200 Subject: [PATCH] [compiler] Make JSObjectRef and others background-serialized .. now that all JSObjectRef methods can run in concurrent settings. Also change a few subtypes to bg-serialized: - JSArray - JSGlobalProxy - JSTypedArray Bug: v8:7790 Change-Id: I406b0a8eacb4e5bd2c3a24eb106b29df2cf55421 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2966377 Reviewed-by: Georg Neis Commit-Queue: Jakob Gruber Cr-Commit-Position: refs/heads/master@{#75452} --- src/compiler/compilation-dependencies.cc | 62 ++++++++++++++---------- src/compiler/heap-refs.cc | 48 +++++++----------- src/compiler/heap-refs.h | 8 +-- 3 files changed, 58 insertions(+), 60 deletions(-) diff --git a/src/compiler/compilation-dependencies.cc b/src/compiler/compilation-dependencies.cc index 81cbb06734..0f957833b8 100644 --- a/src/compiler/compilation-dependencies.cc +++ b/src/compiler/compilation-dependencies.cc @@ -92,11 +92,14 @@ class PrototypePropertyDependency final : public CompilationDependency { class StableMapDependency final : public CompilationDependency { public: - explicit StableMapDependency(const MapRef& map) : map_(map) { - DCHECK(map_.is_stable()); - } + explicit StableMapDependency(const MapRef& map) : map_(map) {} - bool IsValid() const override { return map_.object()->is_stable(); } + bool IsValid() const override { + // TODO(v8:11670): Consider turn this back into a CHECK inside the + // constructor and DependOnStableMap, if possible in light of concurrent + // heap state modifications. + return !map_.object()->is_dictionary_map() && map_.object()->is_stable(); + } void Install(Handle code) const override { SLOW_DCHECK(IsValid()); @@ -396,15 +399,19 @@ class FieldRepresentationDependency final : public CompilationDependency { : owner_(owner), descriptor_(descriptor), representation_(representation) { - CHECK(owner_.equals(owner_.FindFieldOwner(descriptor_))); - CHECK(representation_.Equals( - owner_.GetPropertyDetails(descriptor_).representation())); } bool IsValid() const override { DisallowGarbageCollection no_heap_allocation; Handle owner = owner_.object(); - return representation_.Equals(owner->instance_descriptors(owner_.isolate()) + Isolate* isolate = owner_.isolate(); + + // TODO(v8:11670): Consider turn this back into a CHECK inside the + // constructor, if possible in light of concurrent heap state + // modifications. + if (owner->FindFieldOwner(isolate, descriptor_) != *owner) return false; + + return representation_.Equals(owner->instance_descriptors(isolate) .GetDetails(descriptor_) .representation()); } @@ -434,17 +441,21 @@ class FieldTypeDependency final : public CompilationDependency { // longer need to explicitly store the type. FieldTypeDependency(const MapRef& owner, InternalIndex descriptor, const ObjectRef& type) - : owner_(owner), descriptor_(descriptor), type_(type) { - CHECK(owner_.equals(owner_.FindFieldOwner(descriptor_))); - CHECK(type_.equals(owner_.GetFieldType(descriptor_))); - } + : owner_(owner), descriptor_(descriptor), type_(type) {} bool IsValid() const override { DisallowGarbageCollection no_heap_allocation; Handle owner = owner_.object(); + Isolate* isolate = owner_.isolate(); + + // TODO(v8:11670): Consider turn this back into a CHECK inside the + // constructor, if possible in light of concurrent heap state + // modifications. + if (owner->FindFieldOwner(isolate, descriptor_) != *owner) return false; + Handle type = type_.object(); - return *type == owner->instance_descriptors(owner_.isolate()) - .GetFieldType(descriptor_); + return *type == + owner->instance_descriptors(isolate).GetFieldType(descriptor_); } void Install(Handle code) const override { @@ -462,19 +473,21 @@ class FieldTypeDependency final : public CompilationDependency { class FieldConstnessDependency final : public CompilationDependency { public: FieldConstnessDependency(const MapRef& owner, InternalIndex descriptor) - : owner_(owner), descriptor_(descriptor) { - CHECK(owner_.equals(owner_.FindFieldOwner(descriptor_))); - CHECK_EQ(PropertyConstness::kConst, - owner_.GetPropertyDetails(descriptor_).constness()); - } + : owner_(owner), descriptor_(descriptor) {} bool IsValid() const override { DisallowGarbageCollection no_heap_allocation; Handle owner = owner_.object(); - return PropertyConstness::kConst == - owner->instance_descriptors(owner_.isolate()) - .GetDetails(descriptor_) - .constness(); + Isolate* isolate = owner_.isolate(); + + // TODO(v8:11670): Consider turn this back into a CHECK inside the + // constructor, if possible in light of concurrent heap state + // modifications. + if (owner->FindFieldOwner(isolate, descriptor_) != *owner) return false; + + return PropertyConstness::kConst == owner->instance_descriptors(isolate) + .GetDetails(descriptor_) + .constness(); } void Install(Handle code) const override { @@ -658,12 +671,9 @@ ObjectRef CompilationDependencies::DependOnPrototypeProperty( } void CompilationDependencies::DependOnStableMap(const MapRef& map) { - DCHECK(!map.is_dictionary_map()); DCHECK(!map.IsNeverSerializedHeapObject()); if (map.CanTransition()) { RecordDependency(zone_->New(map)); - } else { - DCHECK(map.is_stable()); } } diff --git a/src/compiler/heap-refs.cc b/src/compiler/heap-refs.cc index 3696b82ea8..2b1bca87bc 100644 --- a/src/compiler/heap-refs.cc +++ b/src/compiler/heap-refs.cc @@ -431,7 +431,8 @@ class JSReceiverData : public HeapObjectData { class JSObjectData : public JSReceiverData { public: JSObjectData(JSHeapBroker* broker, ObjectData** storage, - Handle object); + Handle object, + ObjectDataKind kind = kSerializedHeapObject); // Recursive serialization of all reachable JSObjects. void SerializeAsBoilerplate(JSHeapBroker* broker); @@ -692,12 +693,8 @@ ObjectData* JSObjectData::GetOwnDictionaryProperty(JSHeapBroker* broker, class JSTypedArrayData : public JSObjectData { public: JSTypedArrayData(JSHeapBroker* broker, ObjectData** storage, - Handle object) - : JSObjectData(broker, storage, object) {} - - // TODO(v8:7790): Once JSObject is no longer serialized, also make - // JSTypedArrayRef never-serialized. - STATIC_ASSERT(IsSerializedRef()); + Handle object, ObjectDataKind kind) + : JSObjectData(broker, storage, object, kind) {} void Serialize(JSHeapBroker* broker); bool serialized() const { return serialized_; } @@ -1384,7 +1381,8 @@ HeapObjectData::HeapObjectData(JSHeapBroker* broker, ObjectData** storage, // instance_type_ member. In the case of constructing the MapData for the // meta map (whose map is itself), this member has not yet been // initialized. - map_(broker->GetOrCreateData(object->map(kAcquireLoad))) { + map_(broker->GetOrCreateData(object->map(kAcquireLoad), + kAssumeMemoryFence)) { CHECK_IMPLIES(kind == kSerializedHeapObject, broker->mode() == JSHeapBroker::kSerializing); CHECK_IMPLIES(broker->mode() == JSHeapBroker::kSerialized, @@ -1873,9 +1871,8 @@ bool JSBoundFunctionData::Serialize(JSHeapBroker* broker) { } JSObjectData::JSObjectData(JSHeapBroker* broker, ObjectData** storage, - Handle object) - : JSReceiverData(broker, storage, object, - ObjectDataKind::kSerializedHeapObject), + Handle object, ObjectDataKind kind) + : JSReceiverData(broker, storage, object, kind), inobject_fields_(broker->zone()), own_constant_elements_(broker->zone()), own_properties_(broker->zone()) {} @@ -1907,7 +1904,10 @@ class BytecodeArrayData : public FixedArrayBaseData { class JSArrayData : public JSObjectData { public: JSArrayData(JSHeapBroker* broker, ObjectData** storage, - Handle object); + Handle object, + ObjectDataKind kind = kSerializedHeapObject) + : JSObjectData(broker, storage, object, kind), + own_elements_(broker->zone()) {} void Serialize(JSHeapBroker* broker); ObjectData* length() const { @@ -1930,10 +1930,6 @@ class JSArrayData : public JSObjectData { ZoneVector> own_elements_; }; -JSArrayData::JSArrayData(JSHeapBroker* broker, ObjectData** storage, - Handle object) - : JSObjectData(broker, storage, object), own_elements_(broker->zone()) {} - void JSArrayData::Serialize(JSHeapBroker* broker) { CHECK(!broker->is_concurrent_inlining()); @@ -2217,13 +2213,11 @@ JSGlobalObjectData::JSGlobalObjectData(JSHeapBroker* broker, class JSGlobalProxyData : public JSObjectData { public: JSGlobalProxyData(JSHeapBroker* broker, ObjectData** storage, - Handle object); + Handle object, + ObjectDataKind kind = kSerializedHeapObject) + : JSObjectData(broker, storage, object, kind) {} }; -JSGlobalProxyData::JSGlobalProxyData(JSHeapBroker* broker, ObjectData** storage, - Handle object) - : JSObjectData(broker, storage, object) {} - namespace { base::Optional GetPropertyCellFromHeap(JSHeapBroker* broker, @@ -3640,7 +3634,8 @@ DescriptorArrayRef MapRef::instance_descriptors() const { base::Optional MapRef::prototype() const { if (data_->should_access_heap() || broker()->is_concurrent_inlining()) { - return TryMakeRef(broker(), HeapObject::cast(object()->prototype())); + return MakeRefAssumeMemoryFence(broker(), + HeapObject::cast(object()->prototype())); } ObjectData* prototype_data = data()->AsMap()->prototype(); if (prototype_data == nullptr) { @@ -4612,14 +4607,7 @@ void SourceTextModuleRef::Serialize() { void JSTypedArrayRef::Serialize() { if (data_->should_access_heap() || broker()->is_concurrent_inlining()) { - // 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_ofbuffer())>::value, ""); - STATIC_ASSERT(IsSerializedRef()); - MakeRef(broker(), object()->buffer()); + // Nothing to do. } else { CHECK_EQ(broker()->mode(), JSHeapBroker::kSerializing); data()->AsJSTypedArray()->Serialize(broker()); diff --git a/src/compiler/heap-refs.h b/src/compiler/heap-refs.h index c6efc0b1e8..52d6c337fa 100644 --- a/src/compiler/heap-refs.h +++ b/src/compiler/heap-refs.h @@ -80,13 +80,13 @@ enum class RefSerializationKind { // DO NOT VIOLATE THIS PROPERTY! #define HEAP_BROKER_OBJECT_LIST(V) \ /* Subtypes of JSObject */ \ - V(JSArray, RefSerializationKind::kSerialized) \ + V(JSArray, RefSerializationKind::kBackgroundSerialized) \ V(JSBoundFunction, RefSerializationKind::kSerialized) \ V(JSDataView, RefSerializationKind::kSerialized) \ V(JSFunction, RefSerializationKind::kSerialized) \ V(JSGlobalObject, RefSerializationKind::kSerialized) \ - V(JSGlobalProxy, RefSerializationKind::kSerialized) \ - V(JSTypedArray, RefSerializationKind::kSerialized) \ + V(JSGlobalProxy, RefSerializationKind::kBackgroundSerialized) \ + V(JSTypedArray, RefSerializationKind::kBackgroundSerialized) \ /* Subtypes of Context */ \ V(NativeContext, RefSerializationKind::kNeverSerialized) \ /* Subtypes of FixedArray */ \ @@ -102,7 +102,7 @@ enum class RefSerializationKind { V(String, RefSerializationKind::kNeverSerialized) \ V(Symbol, RefSerializationKind::kNeverSerialized) \ /* Subtypes of JSReceiver */ \ - V(JSObject, RefSerializationKind::kSerialized) \ + V(JSObject, RefSerializationKind::kBackgroundSerialized) \ /* Subtypes of HeapObject */ \ V(AccessorInfo, RefSerializationKind::kNeverSerialized) \ V(AllocationSite, RefSerializationKind::kSerialized) \