Skip to content

Commit 1d797bd

Browse files
rmacnak-googleCommit Queue
authored andcommitted
Revert "[vm, compiler] Treat _uninitializedData/Index as effectively const."
This reverts commit 562af5d. Reason for revert: regression, loses exact type for initial value? Original change's description: > [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 <[email protected]> > Commit-Queue: Ryan Macnak <[email protected]> Change-Id: I41af0247277f6456ba9c5dfcdb8da43cebc64cc0 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/400222 Commit-Queue: Siva Annamalai <[email protected]> Reviewed-by: Siva Annamalai <[email protected]> Auto-Submit: Ryan Macnak <[email protected]> Reviewed-by: Ryan Macnak <[email protected]>
1 parent 752a913 commit 1d797bd

File tree

9 files changed

+10
-60
lines changed

9 files changed

+10
-60
lines changed

runtime/vm/compiler/backend/constant_propagator.cc

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -857,10 +857,9 @@ void ConstantPropagator::VisitLoadIndexedUnsafe(LoadIndexedUnsafeInstr* instr) {
857857
}
858858

859859
void ConstantPropagator::VisitLoadStaticField(LoadStaticFieldInstr* instr) {
860-
// We cannot generally take the current value for an initialized constant
861-
// field because the same code will be used when the AppAOT or AppJIT starts
862-
// over with everything uninitialized or another isolate in the isolate group
863-
// starts with everything uninitialized.
860+
// Cannot treat an initialized field as constant because the same code will be
861+
// used when the AppAOT or AppJIT starts over with everything uninitialized or
862+
// another isolate in the isolate group starts with everything uninitialized.
864863
SetValue(instr, non_constant_);
865864
}
866865

runtime/vm/compiler/backend/il_serializer.cc

Lines changed: 2 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,7 @@ FlowGraphSerializer::FlowGraphSerializer(NonStreamingWriteStream* stream)
3030
zone_(Thread::Current()->zone()),
3131
thread_(Thread::Current()),
3232
isolate_group_(IsolateGroup::Current()),
33-
heap_(IsolateGroup::Current()->heap()) {
34-
// We want to preserve the identity of these, even though they are not const.
35-
AddBaseObject(Object::uninitialized_index());
36-
AddBaseObject(Object::uninitialized_data());
37-
}
33+
heap_(IsolateGroup::Current()->heap()) {}
3834

3935
FlowGraphSerializer::~FlowGraphSerializer() {
4036
heap_->ResetObjectIdTable();
@@ -47,11 +43,7 @@ FlowGraphDeserializer::FlowGraphDeserializer(
4743
stream_(stream),
4844
zone_(Thread::Current()->zone()),
4945
thread_(Thread::Current()),
50-
isolate_group_(IsolateGroup::Current()) {
51-
// We want to preserve the identity of these, even though they are not const.
52-
AddBaseObject(Object::uninitialized_index());
53-
AddBaseObject(Object::uninitialized_data());
54-
}
46+
isolate_group_(IsolateGroup::Current()) {}
5547

5648
ClassPtr FlowGraphDeserializer::GetClassById(classid_t id) const {
5749
return isolate_group()->class_table()->At(id);
@@ -1411,11 +1403,6 @@ void MoveOperands::Write(FlowGraphSerializer* s) const {
14111403
MoveOperands::MoveOperands(FlowGraphDeserializer* d)
14121404
: dest_(Location::Read(d)), src_(Location::Read(d)) {}
14131405

1414-
void FlowGraphSerializer::AddBaseObject(const Object& x) {
1415-
const intptr_t object_index = object_counter_++;
1416-
heap()->SetObjectId(x.ptr(), object_index + 1);
1417-
}
1418-
14191406
template <>
14201407
void FlowGraphSerializer::WriteTrait<const Object&>::Write(
14211408
FlowGraphSerializer* s,
@@ -1436,12 +1423,6 @@ void FlowGraphSerializer::WriteTrait<const Object&>::Write(
14361423
s->WriteObjectImpl(x, cid, object_index);
14371424
}
14381425

1439-
void FlowGraphDeserializer::AddBaseObject(const Object& x) {
1440-
const intptr_t object_index = object_counter_;
1441-
object_counter_++;
1442-
SetObjectAt(object_index, x);
1443-
}
1444-
14451426
template <>
14461427
const Object& FlowGraphDeserializer::ReadTrait<const Object&>::Read(
14471428
FlowGraphDeserializer* d) {

runtime/vm/compiler/backend/il_serializer.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,6 @@ class FlowGraphSerializer : public ValueObject {
304304
bool can_write_refs() const { return can_write_refs_; }
305305

306306
private:
307-
void AddBaseObject(const Object& x);
308307
void WriteObjectImpl(const Object& x, intptr_t cid, intptr_t object_index);
309308
bool IsWritten(const Object& obj);
310309
bool HasEnclosingTypes(const Object& obj);
@@ -498,7 +497,6 @@ class FlowGraphDeserializer : public ValueObject {
498497
}
499498

500499
private:
501-
void AddBaseObject(const Object& x);
502500
ClassPtr GetClassById(classid_t id) const;
503501
const Object& ReadObjectImpl(intptr_t cid, intptr_t object_index);
504502
void SetObjectAt(intptr_t object_index, const Object& object);

runtime/vm/compiler/frontend/kernel_to_il.cc

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1115,8 +1115,6 @@ bool FlowGraphBuilder::IsRecognizedMethodForFlowGraph(
11151115
case MethodRecognizer::kUtf8DecoderScan:
11161116
case MethodRecognizer::kHas63BitSmis:
11171117
case MethodRecognizer::kExtensionStreamHasListener:
1118-
case MethodRecognizer::kCompactHash_uninitializedIndex:
1119-
case MethodRecognizer::kCompactHash_uninitializedData:
11201118
case MethodRecognizer::kSmi_hashCode:
11211119
case MethodRecognizer::kMint_hashCode:
11221120
case MethodRecognizer::kDouble_hashCode:
@@ -1728,12 +1726,6 @@ FlowGraph* FlowGraphBuilder::BuildGraphOfRecognizedMethod(
17281726
body += IntToBool();
17291727
#endif // PRODUCT
17301728
} break;
1731-
case MethodRecognizer::kCompactHash_uninitializedIndex: {
1732-
body += Constant(Object::uninitialized_index());
1733-
} break;
1734-
case MethodRecognizer::kCompactHash_uninitializedData: {
1735-
body += Constant(Object::uninitialized_data());
1736-
} break;
17371729
case MethodRecognizer::kSmi_hashCode: {
17381730
// TODO(dartbug.com/38985): We should make this LoadLocal+Unbox+
17391731
// IntegerHash+Box. Though this would make use of unboxed values on stack

runtime/vm/compiler/recognized_methods_list.h

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -101,10 +101,6 @@ namespace dart {
101101
ImmutableLinkedHashBase_getIndex, 0xfe7649ae) \
102102
V(CompactHashLibrary, _HashVMImmutableBase, set:_index, \
103103
ImmutableLinkedHashBase_setIndexStoreRelease, 0xcf36944c) \
104-
V(CompactHashLibrary, ::, get:_uninitializedIndex, \
105-
CompactHash_uninitializedIndex, 0xa25a7625) \
106-
V(CompactHashLibrary, ::, get:_uninitializedData, \
107-
CompactHash_uninitializedData, 0x06a5677a) \
108104
V(DeveloperLibrary, ::, get:extensionStreamHasListener, \
109105
ExtensionStreamHasListener, 0xfa975305) \
110106
V(DeveloperLibrary, ::, debugger, Debugger, 0xf0aaff14) \

runtime/vm/object.cc

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1329,11 +1329,6 @@ void Object::Init(IsolateGroup* isolate_group) {
13291329
KernelBytecode::kVMInternal_ImplicitConstructorClosure);
13301330
#endif // defined(DART_DYNAMIC_MODULES)
13311331

1332-
*uninitialized_index_ =
1333-
TypedData::New(kTypedDataUint32ArrayCid,
1334-
LinkedHashBase::kUninitializedIndexSize, Heap::kOld);
1335-
*uninitialized_data_ = Array::New(0, Heap::kOld);
1336-
13371332
// Some thread fields need to be reinitialized as null constants have not been
13381333
// initialized until now.
13391334
thread->ClearStickyError();
@@ -1433,10 +1428,6 @@ void Object::Init(IsolateGroup* isolate_group) {
14331428
ASSERT(implicit_instance_closure_bytecode_->IsBytecode());
14341429
ASSERT(!implicit_constructor_closure_bytecode_->IsSmi());
14351430
ASSERT(implicit_constructor_closure_bytecode_->IsBytecode());
1436-
ASSERT(!uninitialized_index_->IsSmi());
1437-
ASSERT(uninitialized_index_->IsTypedData());
1438-
ASSERT(!uninitialized_data_->IsSmi());
1439-
ASSERT(uninitialized_data_->IsArray());
14401431
}
14411432

14421433
void Object::FinishInit(IsolateGroup* isolate_group) {

runtime/vm/object.h

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -517,9 +517,7 @@ class Object {
517517
V(Array, vm_isolate_snapshot_object_table) \
518518
V(Type, dynamic_type) \
519519
V(Type, void_type) \
520-
V(AbstractType, null_abstract_type) \
521-
V(TypedData, uninitialized_index) \
522-
V(Array, uninitialized_data)
520+
V(AbstractType, null_abstract_type)
523521

524522
#define DEFINE_SHARED_READONLY_HANDLE_GETTER(Type, name) \
525523
static const Type& name() { \
@@ -12149,10 +12147,10 @@ class LinkedHashBase : public Instance {
1214912147
virtual uint32_t CanonicalizeHash() const;
1215012148
virtual void CanonicalizeFieldsLocked(Thread* thread) const;
1215112149

12150+
protected:
1215212151
// Keep this in sync with Dart implementation (lib/compact_hash.dart).
1215312152
static constexpr intptr_t kInitialIndexBits = 2;
1215412153
static constexpr intptr_t kInitialIndexSize = 1 << (kInitialIndexBits + 1);
12155-
static constexpr intptr_t kUninitializedIndexSize = 1;
1215612154

1215712155
private:
1215812156
LinkedHashBasePtr ptr() const { return static_cast<LinkedHashBasePtr>(ptr_); }

runtime/vm/object_store.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -528,7 +528,7 @@ class ObjectStore {
528528
#undef DECLARE_LAZY_INIT_ASYNC_GETTER
529529
#undef DECLARE_LAZY_INIT_ISOLATE_GETTER
530530
#undef DECLARE_LAZY_INIT_INTERNAL_GETTER
531-
#undef DECLARE_LAZY_INIT_FFI_GETTER
531+
#undef DECLARE_LAZY_INIT_TYPED_DATA_GETTER
532532

533533
LibraryPtr bootstrap_library(BootstrapLibraryId index) {
534534
switch (index) {

sdk/lib/_internal/vm_shared/lib/compact_hash.dart

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -360,16 +360,11 @@ mixin _CustomEqualsAndHashCode<K> implements _EqualsAndHashCode {
360360
bool _equals(Object? e1, Object? e2) => (_equality as Function)(e1, e2);
361361
}
362362

363-
@pragma("vm:prefer-inline")
364-
@pragma("vm:recognized", "other")
365-
external Uint32List get _uninitializedIndex;
366-
363+
final _uninitializedIndex = new Uint32List(_HashBase._UNINITIALIZED_INDEX_SIZE);
367364
// Note: not const. Const arrays are made immutable by having a different class
368365
// than regular arrays that throws on element assignment. We want the data field
369366
// in maps and sets to be monomorphic.
370-
@pragma("vm:prefer-inline")
371-
@pragma("vm:recognized", "other")
372-
external List<Object?> get _uninitializedData;
367+
final _uninitializedData = new List.filled(0, null);
373368

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

0 commit comments

Comments
 (0)