From 562af5de3562584dd15a115d27808cfb14eefc85 Mon Sep 17 00:00:00 2001 From: Ryan Macnak Date: Wed, 11 Dec 2024 14:46:58 -0800 Subject: [PATCH] [vm, compiler] Treat _uninitializedData/Index as effectively const. This avoids static field initialization checks in the default map and set constructors, which in turn avoids write barriers and the frame build. TEST=ci Change-Id: Ie12840ae1799cd97645b2132ad94ec6525126f74 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/398760 Reviewed-by: Alexander Markov Commit-Queue: Ryan Macnak --- .../compiler/backend/constant_propagator.cc | 7 +++--- runtime/vm/compiler/backend/il_serializer.cc | 23 +++++++++++++++++-- runtime/vm/compiler/backend/il_serializer.h | 2 ++ runtime/vm/compiler/frontend/kernel_to_il.cc | 8 +++++++ runtime/vm/compiler/recognized_methods_list.h | 4 ++++ runtime/vm/object.cc | 9 ++++++++ runtime/vm/object.h | 6 +++-- runtime/vm/object_store.h | 2 +- .../_internal/vm_shared/lib/compact_hash.dart | 9 ++++++-- 9 files changed, 60 insertions(+), 10 deletions(-) diff --git a/runtime/vm/compiler/backend/constant_propagator.cc b/runtime/vm/compiler/backend/constant_propagator.cc index f56846fbbaab..8bba94712db9 100644 --- a/runtime/vm/compiler/backend/constant_propagator.cc +++ b/runtime/vm/compiler/backend/constant_propagator.cc @@ -857,9 +857,10 @@ void ConstantPropagator::VisitLoadIndexedUnsafe(LoadIndexedUnsafeInstr* instr) { } void ConstantPropagator::VisitLoadStaticField(LoadStaticFieldInstr* instr) { - // Cannot treat an initialized field as constant because the same code will be - // used when the AppAOT or AppJIT starts over with everything uninitialized or - // another isolate in the isolate group starts with everything uninitialized. + // We cannot generally take the current value for an initialized constant + // field because the same code will be used when the AppAOT or AppJIT starts + // over with everything uninitialized or another isolate in the isolate group + // starts with everything uninitialized. SetValue(instr, non_constant_); } diff --git a/runtime/vm/compiler/backend/il_serializer.cc b/runtime/vm/compiler/backend/il_serializer.cc index 552c010c9e24..9c74a54e83cc 100644 --- a/runtime/vm/compiler/backend/il_serializer.cc +++ b/runtime/vm/compiler/backend/il_serializer.cc @@ -30,7 +30,11 @@ FlowGraphSerializer::FlowGraphSerializer(NonStreamingWriteStream* stream) zone_(Thread::Current()->zone()), thread_(Thread::Current()), isolate_group_(IsolateGroup::Current()), - heap_(IsolateGroup::Current()->heap()) {} + heap_(IsolateGroup::Current()->heap()) { + // We want to preserve the identity of these, even though they are not const. + AddBaseObject(Object::uninitialized_index()); + AddBaseObject(Object::uninitialized_data()); +} FlowGraphSerializer::~FlowGraphSerializer() { heap_->ResetObjectIdTable(); @@ -43,7 +47,11 @@ FlowGraphDeserializer::FlowGraphDeserializer( stream_(stream), zone_(Thread::Current()->zone()), thread_(Thread::Current()), - isolate_group_(IsolateGroup::Current()) {} + isolate_group_(IsolateGroup::Current()) { + // We want to preserve the identity of these, even though they are not const. + AddBaseObject(Object::uninitialized_index()); + AddBaseObject(Object::uninitialized_data()); +} ClassPtr FlowGraphDeserializer::GetClassById(classid_t id) const { return isolate_group()->class_table()->At(id); @@ -1403,6 +1411,11 @@ void MoveOperands::Write(FlowGraphSerializer* s) const { MoveOperands::MoveOperands(FlowGraphDeserializer* d) : dest_(Location::Read(d)), src_(Location::Read(d)) {} +void FlowGraphSerializer::AddBaseObject(const Object& x) { + const intptr_t object_index = object_counter_++; + heap()->SetObjectId(x.ptr(), object_index + 1); +} + template <> void FlowGraphSerializer::WriteTrait::Write( FlowGraphSerializer* s, @@ -1423,6 +1436,12 @@ void FlowGraphSerializer::WriteTrait::Write( s->WriteObjectImpl(x, cid, object_index); } +void FlowGraphDeserializer::AddBaseObject(const Object& x) { + const intptr_t object_index = object_counter_; + object_counter_++; + SetObjectAt(object_index, x); +} + template <> const Object& FlowGraphDeserializer::ReadTrait::Read( FlowGraphDeserializer* d) { diff --git a/runtime/vm/compiler/backend/il_serializer.h b/runtime/vm/compiler/backend/il_serializer.h index 13d448cd5a94..cd9365616cb6 100644 --- a/runtime/vm/compiler/backend/il_serializer.h +++ b/runtime/vm/compiler/backend/il_serializer.h @@ -304,6 +304,7 @@ class FlowGraphSerializer : public ValueObject { bool can_write_refs() const { return can_write_refs_; } private: + void AddBaseObject(const Object& x); void WriteObjectImpl(const Object& x, intptr_t cid, intptr_t object_index); bool IsWritten(const Object& obj); bool HasEnclosingTypes(const Object& obj); @@ -497,6 +498,7 @@ class FlowGraphDeserializer : public ValueObject { } private: + void AddBaseObject(const Object& x); ClassPtr GetClassById(classid_t id) const; const Object& ReadObjectImpl(intptr_t cid, intptr_t object_index); void SetObjectAt(intptr_t object_index, const Object& object); diff --git a/runtime/vm/compiler/frontend/kernel_to_il.cc b/runtime/vm/compiler/frontend/kernel_to_il.cc index 96f8006595d7..8493aa388ac5 100644 --- a/runtime/vm/compiler/frontend/kernel_to_il.cc +++ b/runtime/vm/compiler/frontend/kernel_to_il.cc @@ -1115,6 +1115,8 @@ bool FlowGraphBuilder::IsRecognizedMethodForFlowGraph( case MethodRecognizer::kUtf8DecoderScan: case MethodRecognizer::kHas63BitSmis: case MethodRecognizer::kExtensionStreamHasListener: + case MethodRecognizer::kCompactHash_uninitializedIndex: + case MethodRecognizer::kCompactHash_uninitializedData: case MethodRecognizer::kSmi_hashCode: case MethodRecognizer::kMint_hashCode: case MethodRecognizer::kDouble_hashCode: @@ -1726,6 +1728,12 @@ FlowGraph* FlowGraphBuilder::BuildGraphOfRecognizedMethod( body += IntToBool(); #endif // PRODUCT } break; + case MethodRecognizer::kCompactHash_uninitializedIndex: { + body += Constant(Object::uninitialized_index()); + } break; + case MethodRecognizer::kCompactHash_uninitializedData: { + body += Constant(Object::uninitialized_data()); + } break; case MethodRecognizer::kSmi_hashCode: { // TODO(dartbug.com/38985): We should make this LoadLocal+Unbox+ // IntegerHash+Box. Though this would make use of unboxed values on stack diff --git a/runtime/vm/compiler/recognized_methods_list.h b/runtime/vm/compiler/recognized_methods_list.h index b0aba41d3211..59e7a44e6689 100644 --- a/runtime/vm/compiler/recognized_methods_list.h +++ b/runtime/vm/compiler/recognized_methods_list.h @@ -101,6 +101,10 @@ namespace dart { ImmutableLinkedHashBase_getIndex, 0xfe7649ae) \ V(CompactHashLibrary, _HashVMImmutableBase, set:_index, \ ImmutableLinkedHashBase_setIndexStoreRelease, 0xcf36944c) \ + V(CompactHashLibrary, ::, get:_uninitializedIndex, \ + CompactHash_uninitializedIndex, 0xa25a7625) \ + V(CompactHashLibrary, ::, get:_uninitializedData, \ + CompactHash_uninitializedData, 0x06a5677a) \ V(DeveloperLibrary, ::, get:extensionStreamHasListener, \ ExtensionStreamHasListener, 0xfa975305) \ V(DeveloperLibrary, ::, debugger, Debugger, 0xf0aaff14) \ diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc index 75c99e981c71..93369cf26186 100644 --- a/runtime/vm/object.cc +++ b/runtime/vm/object.cc @@ -1329,6 +1329,11 @@ void Object::Init(IsolateGroup* isolate_group) { KernelBytecode::kVMInternal_ImplicitConstructorClosure); #endif // defined(DART_DYNAMIC_MODULES) + *uninitialized_index_ = + TypedData::New(kTypedDataUint32ArrayCid, + LinkedHashBase::kUninitializedIndexSize, Heap::kOld); + *uninitialized_data_ = Array::New(0, Heap::kOld); + // Some thread fields need to be reinitialized as null constants have not been // initialized until now. thread->ClearStickyError(); @@ -1428,6 +1433,10 @@ void Object::Init(IsolateGroup* isolate_group) { ASSERT(implicit_instance_closure_bytecode_->IsBytecode()); ASSERT(!implicit_constructor_closure_bytecode_->IsSmi()); ASSERT(implicit_constructor_closure_bytecode_->IsBytecode()); + ASSERT(!uninitialized_index_->IsSmi()); + ASSERT(uninitialized_index_->IsTypedData()); + ASSERT(!uninitialized_data_->IsSmi()); + ASSERT(uninitialized_data_->IsArray()); } void Object::FinishInit(IsolateGroup* isolate_group) { diff --git a/runtime/vm/object.h b/runtime/vm/object.h index 9cb22bbf9f5e..54d73a0116ad 100644 --- a/runtime/vm/object.h +++ b/runtime/vm/object.h @@ -517,7 +517,9 @@ class Object { V(Array, vm_isolate_snapshot_object_table) \ V(Type, dynamic_type) \ V(Type, void_type) \ - V(AbstractType, null_abstract_type) + V(AbstractType, null_abstract_type) \ + V(TypedData, uninitialized_index) \ + V(Array, uninitialized_data) #define DEFINE_SHARED_READONLY_HANDLE_GETTER(Type, name) \ static const Type& name() { \ @@ -12147,10 +12149,10 @@ class LinkedHashBase : public Instance { virtual uint32_t CanonicalizeHash() const; virtual void CanonicalizeFieldsLocked(Thread* thread) const; - protected: // Keep this in sync with Dart implementation (lib/compact_hash.dart). static constexpr intptr_t kInitialIndexBits = 2; static constexpr intptr_t kInitialIndexSize = 1 << (kInitialIndexBits + 1); + static constexpr intptr_t kUninitializedIndexSize = 1; private: LinkedHashBasePtr ptr() const { return static_cast(ptr_); } diff --git a/runtime/vm/object_store.h b/runtime/vm/object_store.h index f120dea22b5e..fedffda27133 100644 --- a/runtime/vm/object_store.h +++ b/runtime/vm/object_store.h @@ -528,7 +528,7 @@ class ObjectStore { #undef DECLARE_LAZY_INIT_ASYNC_GETTER #undef DECLARE_LAZY_INIT_ISOLATE_GETTER #undef DECLARE_LAZY_INIT_INTERNAL_GETTER -#undef DECLARE_LAZY_INIT_TYPED_DATA_GETTER +#undef DECLARE_LAZY_INIT_FFI_GETTER LibraryPtr bootstrap_library(BootstrapLibraryId index) { switch (index) { diff --git a/sdk/lib/_internal/vm_shared/lib/compact_hash.dart b/sdk/lib/_internal/vm_shared/lib/compact_hash.dart index 4db048099bc7..2f97f3fa3601 100644 --- a/sdk/lib/_internal/vm_shared/lib/compact_hash.dart +++ b/sdk/lib/_internal/vm_shared/lib/compact_hash.dart @@ -360,11 +360,16 @@ mixin _CustomEqualsAndHashCode implements _EqualsAndHashCode { bool _equals(Object? e1, Object? e2) => (_equality as Function)(e1, e2); } -final _uninitializedIndex = new Uint32List(_HashBase._UNINITIALIZED_INDEX_SIZE); +@pragma("vm:prefer-inline") +@pragma("vm:recognized", "other") +external Uint32List get _uninitializedIndex; + // Note: not const. Const arrays are made immutable by having a different class // than regular arrays that throws on element assignment. We want the data field // in maps and sets to be monomorphic. -final _uninitializedData = new List.filled(0, null); +@pragma("vm:prefer-inline") +@pragma("vm:recognized", "other") +external List get _uninitializedData; // VM-internalized implementation of a default-constructed LinkedHashMap. Map // literals also create instances of this class.