Skip to content

Commit

Permalink
[vm, compiler] Treat _uninitializedData/Index as effectively const.
Browse files Browse the repository at this point in the history
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 <[email protected]>
Commit-Queue: Ryan Macnak <[email protected]>
  • Loading branch information
rmacnak-google authored and Commit Queue committed Dec 11, 2024
1 parent 0ce6724 commit 562af5d
Show file tree
Hide file tree
Showing 9 changed files with 60 additions and 10 deletions.
7 changes: 4 additions & 3 deletions runtime/vm/compiler/backend/constant_propagator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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_);
}

Expand Down
23 changes: 21 additions & 2 deletions runtime/vm/compiler/backend/il_serializer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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);
Expand Down Expand Up @@ -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<const Object&>::Write(
FlowGraphSerializer* s,
Expand All @@ -1423,6 +1436,12 @@ void FlowGraphSerializer::WriteTrait<const Object&>::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<const Object&>::Read(
FlowGraphDeserializer* d) {
Expand Down
2 changes: 2 additions & 0 deletions runtime/vm/compiler/backend/il_serializer.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
8 changes: 8 additions & 0 deletions runtime/vm/compiler/frontend/kernel_to_il.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions runtime/vm/compiler/recognized_methods_list.h
Original file line number Diff line number Diff line change
Expand Up @@ -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) \
Expand Down
9 changes: 9 additions & 0 deletions runtime/vm/object.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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) {
Expand Down
6 changes: 4 additions & 2 deletions runtime/vm/object.h
Original file line number Diff line number Diff line change
Expand Up @@ -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() { \
Expand Down Expand Up @@ -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<LinkedHashBasePtr>(ptr_); }
Expand Down
2 changes: 1 addition & 1 deletion runtime/vm/object_store.h
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
9 changes: 7 additions & 2 deletions sdk/lib/_internal/vm_shared/lib/compact_hash.dart
Original file line number Diff line number Diff line change
Expand Up @@ -360,11 +360,16 @@ mixin _CustomEqualsAndHashCode<K> 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<Object?> get _uninitializedData;

// VM-internalized implementation of a default-constructed LinkedHashMap. Map
// literals also create instances of this class.
Expand Down

0 comments on commit 562af5d

Please sign in to comment.