Skip to content

Commit 3675fd2

Browse files
alexmarkovCommit Queue
alexmarkov
authored and
Commit Queue
committed
[vm] Packed representation of record shape
The representation of record shape in record instances and record types is changed from a pair int num_fields Array field_names to a single integer - packed bitfield int num_fields(16) int field_names_index(kSmiBits-16) where field names index is an index in the array available from ObjectStore. With the new representation of record shapes: 1) Size of record instances is reduced. 2) Number of comparisons for a shape test reduced from 2 to 1 (shape test is used during type checks). 3) A few operations removed from Record.hashCode. 4) Type testing stubs (TTS) are now supported for record types with named fields. Previously it was not possible to check shape of records with named fields in TTS as TTS cannot access object pool and cannot load field names array). TEST=existing Issue: #49719 Change-Id: I7cdcbb53938aba5d561cd24dc99530395dbbea7e Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/276201 Reviewed-by: Ryan Macnak <[email protected]> Commit-Queue: Alexander Markov <[email protected]>
1 parent e38a704 commit 3675fd2

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

52 files changed

+1457
-1104
lines changed

‎runtime/lib/object.cc

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -148,13 +148,12 @@ static bool HaveSameRuntimeTypeHelper(Zone* zone,
148148
if (left_cid == kRecordCid) {
149149
const auto& left_record = Record::Cast(left);
150150
const auto& right_record = Record::Cast(right);
151-
const intptr_t num_fields = left_record.num_fields();
152-
if ((num_fields != right_record.num_fields()) ||
153-
(left_record.field_names() != right_record.field_names())) {
151+
if (left_record.shape() != right_record.shape()) {
154152
return false;
155153
}
156154
Instance& left_field = Instance::Handle(zone);
157155
Instance& right_field = Instance::Handle(zone);
156+
const intptr_t num_fields = left_record.num_fields();
158157
for (intptr_t i = 0; i < num_fields; ++i) {
159158
left_field ^= left_record.FieldAt(i);
160159
right_field ^= right_record.FieldAt(i);

‎runtime/tests/vm/dart/records_return_value_unboxing_il_test.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ void matchIL$test(FlowGraph graph) {
138138
'r4' << match.DispatchTableCall('obj2_cid'),
139139
'r4_0' << match.ExtractNthOutput('r4', index: 0),
140140
'r4_y' << match.ExtractNthOutput('r4', index: 1),
141-
'r4_boxed' << match.AllocateSmallRecord(match.any, 'r4_0', 'r4_y'),
141+
'r4_boxed' << match.AllocateSmallRecord('r4_0', 'r4_y'),
142142
match.PushArgument('r4_boxed'),
143143
match.StaticCall(),
144144

‎runtime/vm/app_snapshot.cc

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4934,8 +4934,7 @@ class RecordSerializationCluster : public SerializationCluster {
49344934
RecordPtr record = Record::RawCast(object);
49354935
objects_.Add(record);
49364936

4937-
s->Push(record->untag()->field_names());
4938-
const intptr_t num_fields = Smi::Value(record->untag()->num_fields());
4937+
const intptr_t num_fields = Record::NumFields(record);
49394938
for (intptr_t i = 0; i < num_fields; ++i) {
49404939
s->Push(record->untag()->field(i));
49414940
}
@@ -4948,7 +4947,7 @@ class RecordSerializationCluster : public SerializationCluster {
49484947
RecordPtr record = objects_[i];
49494948
s->AssignRef(record);
49504949
AutoTraceObject(record);
4951-
const intptr_t num_fields = Smi::Value(record->untag()->num_fields());
4950+
const intptr_t num_fields = Record::NumFields(record);
49524951
s->WriteUnsigned(num_fields);
49534952
target_memory_size_ += compiler::target::Record::InstanceSize(num_fields);
49544953
}
@@ -4959,9 +4958,9 @@ class RecordSerializationCluster : public SerializationCluster {
49594958
for (intptr_t i = 0; i < count; ++i) {
49604959
RecordPtr record = objects_[i];
49614960
AutoTraceObject(record);
4962-
const intptr_t num_fields = Smi::Value(record->untag()->num_fields());
4963-
s->WriteUnsigned(num_fields);
4964-
WriteField(record, field_names());
4961+
const RecordShape shape(record->untag()->shape());
4962+
s->WriteUnsigned(shape.AsInt());
4963+
const intptr_t num_fields = shape.num_fields();
49654964
for (intptr_t j = 0; j < num_fields; ++j) {
49664965
s->WriteElementRef(record->untag()->field(j), j);
49674966
}
@@ -4998,12 +4997,12 @@ class RecordDeserializationCluster
49984997
const bool stamp_canonical = primary && is_canonical();
49994998
for (intptr_t id = start_index_, n = stop_index_; id < n; id++) {
50004999
RecordPtr record = static_cast<RecordPtr>(d.Ref(id));
5001-
const intptr_t num_fields = d.ReadUnsigned();
5000+
const intptr_t shape = d.ReadUnsigned();
5001+
const intptr_t num_fields = RecordShape(shape).num_fields();
50025002
Deserializer::InitializeHeader(record, kRecordCid,
50035003
Record::InstanceSize(num_fields),
50045004
stamp_canonical);
5005-
record->untag()->num_fields_ = Smi::New(num_fields);
5006-
record->untag()->field_names_ = static_cast<ArrayPtr>(d.ReadRef());
5005+
record->untag()->shape_ = Smi::New(shape);
50075006
for (intptr_t j = 0; j < num_fields; ++j) {
50085007
record->untag()->data()[j] = d.ReadRef();
50095008
}

‎runtime/vm/class_finalizer.cc

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -940,11 +940,6 @@ AbstractTypePtr ClassFinalizer::FinalizeRecordType(
940940
record.SetFieldTypeAt(i, finalized_type);
941941
}
942942
}
943-
// Canonicalize field names so they can be compared with pointer comparison.
944-
// The field names are already sorted in the front-end.
945-
Array& field_names = Array::Handle(zone, record.field_names());
946-
field_names ^= field_names.Canonicalize(Thread::Current());
947-
record.set_field_names(field_names);
948943

949944
if (FLAG_trace_type_finalization) {
950945
THR_Print("Marking record type '%s' as finalized\n",

‎runtime/vm/compiler/assembler/assembler_arm.h

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -854,12 +854,17 @@ class Assembler : public AssemblerBase {
854854
void AddRegisters(Register dest, Register src) {
855855
add(dest, dest, Operand(src));
856856
}
857+
// [dest] = [src] << [scale] + [value].
857858
void AddScaled(Register dest,
858859
Register src,
859860
ScaleFactor scale,
860861
int32_t value) {
861-
LoadImmediate(dest, value);
862-
add(dest, dest, Operand(src, LSL, scale));
862+
if (scale == 0) {
863+
AddImmediate(dest, src, value);
864+
} else {
865+
Lsl(dest, src, Operand(scale));
866+
AddImmediate(dest, dest, value);
867+
}
863868
}
864869
void SubImmediate(Register rd,
865870
Register rn,

‎runtime/vm/compiler/assembler/assembler_arm64.h

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1824,12 +1824,17 @@ class Assembler : public AssemblerBase {
18241824
void AddRegisters(Register dest, Register src) {
18251825
add(dest, dest, Operand(src));
18261826
}
1827+
// [dest] = [src] << [scale] + [value].
18271828
void AddScaled(Register dest,
18281829
Register src,
18291830
ScaleFactor scale,
18301831
int32_t value) {
1831-
LoadImmediate(dest, value);
1832-
add(dest, dest, Operand(src, LSL, scale));
1832+
if (scale == 0) {
1833+
AddImmediate(dest, src, value);
1834+
} else {
1835+
orr(dest, ZR, Operand(src, LSL, scale));
1836+
AddImmediate(dest, dest, value);
1837+
}
18331838
}
18341839
void SubImmediateSetFlags(Register dest,
18351840
Register rn,

‎runtime/vm/compiler/assembler/assembler_ia32.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,7 @@ class Assembler : public AssemblerBase {
254254
void pushl(Register reg);
255255
void pushl(const Address& address);
256256
void pushl(const Immediate& imm);
257+
void PushImmediate(int32_t value) { pushl(Immediate(value)); }
257258

258259
void popl(Register reg);
259260
void popl(const Address& address);
@@ -751,6 +752,7 @@ class Assembler : public AssemblerBase {
751752
void AddRegisters(Register dest, Register src) {
752753
addl(dest, src);
753754
}
755+
// [dest] = [src] << [scale] + [value].
754756
void AddScaled(Register dest,
755757
Register src,
756758
ScaleFactor scale,
@@ -774,6 +776,10 @@ class Assembler : public AssemblerBase {
774776
void AndImmediate(Register dst, int32_t value) {
775777
andl(dst, Immediate(value));
776778
}
779+
void AndImmediate(Register dst, Register src, int32_t value) {
780+
MoveRegister(dst, src);
781+
andl(dst, Immediate(value));
782+
}
777783
void AndRegisters(Register dst,
778784
Register src1,
779785
Register src2 = kNoRegister) override;

‎runtime/vm/compiler/assembler/assembler_riscv.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1009,6 +1009,7 @@ class Assembler : public MicroAssembler {
10091009
MulImmediate(dest, dest, imm, width);
10101010
}
10111011
void AddRegisters(Register dest, Register src) { add(dest, dest, src); }
1012+
// [dest] = [src] << [scale] + [value].
10121013
void AddScaled(Register dest,
10131014
Register src,
10141015
ScaleFactor scale,

‎runtime/vm/compiler/assembler/assembler_x64.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -586,6 +586,10 @@ class Assembler : public AssemblerBase {
586586
void AndImmediate(Register dst, int64_t value) {
587587
AndImmediate(dst, Immediate(value));
588588
}
589+
void AndImmediate(Register dst, Register src, int64_t value) {
590+
MoveRegister(dst, src);
591+
AndImmediate(dst, value);
592+
}
589593
void AndRegisters(Register dst,
590594
Register src1,
591595
Register src2 = kNoRegister) override;
@@ -783,6 +787,7 @@ class Assembler : public AssemblerBase {
783787
void AddRegisters(Register dest, Register src) {
784788
addq(dest, src);
785789
}
790+
// [dest] = [src] << [scale] + [value].
786791
void AddScaled(Register dest,
787792
Register src,
788793
ScaleFactor scale,

‎runtime/vm/compiler/backend/flow_graph.cc

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2000,18 +2000,14 @@ void FlowGraph::InsertRecordBoxing(Definition* def) {
20002000
ASSERT(target != nullptr && !target->IsNull());
20012001
const auto& type = AbstractType::Handle(Z, target->result_type());
20022002
ASSERT(type.IsRecordType());
2003-
const auto& field_names =
2004-
Array::Handle(Z, RecordType::Cast(type).field_names());
2005-
Value* field_names_value = (field_names.Length() != 0)
2006-
? new (Z) Value(GetConstant(field_names))
2007-
: nullptr;
2003+
const RecordShape shape = RecordType::Cast(type).shape();
20082004
auto* x = new (Z)
20092005
ExtractNthOutputInstr(new (Z) Value(def), 0, kTagged, kDynamicCid);
20102006
auto* y = new (Z)
20112007
ExtractNthOutputInstr(new (Z) Value(def), 1, kTagged, kDynamicCid);
2012-
auto* alloc = new (Z) AllocateSmallRecordInstr(
2013-
InstructionSource(), 2, field_names_value, new (Z) Value(x),
2014-
new (Z) Value(y), nullptr, def->deopt_id());
2008+
auto* alloc = new (Z)
2009+
AllocateSmallRecordInstr(InstructionSource(), shape, new (Z) Value(x),
2010+
new (Z) Value(y), nullptr, def->deopt_id());
20152011
def->ReplaceUsesWith(alloc);
20162012
// Uses of 'def' in 'x' and 'y' should not be replaced as 'x' and 'y'
20172013
// are not added to the flow graph yet.

‎runtime/vm/compiler/backend/il.cc

Lines changed: 20 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -395,16 +395,6 @@ bool HierarchyInfo::CanUseRecordSubtypeRangeCheckFor(const AbstractType& type) {
395395
return false;
396396
}
397397
const RecordType& rec = RecordType::Cast(type);
398-
// Type testing stubs have no access to their object pools
399-
// so they will not be able to load field names from object pool
400-
// in order to check the shape of a record instance.
401-
// See TypeTestingStubGenerator::BuildOptimizedRecordSubtypeRangeCheck.
402-
if (rec.NumNamedFields() != 0) {
403-
return false;
404-
} else {
405-
ASSERT(rec.field_names() == Object::empty_array().ptr());
406-
ASSERT(compiler::target::CanLoadFromThread(Object::empty_array()));
407-
}
408398
Zone* zone = thread()->zone();
409399
auto& field_type = AbstractType::Handle(zone);
410400
for (intptr_t i = 0, n = rec.NumFields(); i < n; ++i) {
@@ -2708,16 +2698,9 @@ bool LoadFieldInstr::TryEvaluateLoad(const Object& instance,
27082698
}
27092699
return false;
27102700

2711-
case Slot::Kind::kRecord_num_fields:
2701+
case Slot::Kind::kRecord_shape:
27122702
if (instance.IsRecord()) {
2713-
*result = Smi::New(Record::Cast(instance).num_fields());
2714-
return true;
2715-
}
2716-
return false;
2717-
2718-
case Slot::Kind::kRecord_field_names:
2719-
if (instance.IsRecord()) {
2720-
*result = Record::Cast(instance).field_names();
2703+
*result = Record::Cast(instance).shape().AsSmi();
27212704
return true;
27222705
}
27232706
return false;
@@ -2847,37 +2830,17 @@ Definition* LoadFieldInstr::Canonicalize(FlowGraph* flow_graph) {
28472830
}
28482831
}
28492832
break;
2850-
case Slot::Kind::kRecord_num_fields:
2833+
case Slot::Kind::kRecord_shape:
28512834
ASSERT(!calls_initializer());
28522835
if (auto* alloc_rec = orig_instance->AsAllocateRecord()) {
2853-
return flow_graph->GetConstant(
2854-
Smi::Handle(Smi::New(alloc_rec->num_fields())));
2836+
return flow_graph->GetConstant(Smi::Handle(alloc_rec->shape().AsSmi()));
28552837
} else if (auto* alloc_rec = orig_instance->AsAllocateSmallRecord()) {
2856-
return flow_graph->GetConstant(
2857-
Smi::Handle(Smi::New(alloc_rec->num_fields())));
2838+
return flow_graph->GetConstant(Smi::Handle(alloc_rec->shape().AsSmi()));
28582839
} else {
28592840
const AbstractType* type = instance()->Type()->ToAbstractType();
28602841
if (type->IsRecordType()) {
28612842
return flow_graph->GetConstant(
2862-
Smi::Handle(Smi::New(RecordType::Cast(*type).NumFields())));
2863-
}
2864-
}
2865-
break;
2866-
case Slot::Kind::kRecord_field_names:
2867-
ASSERT(!calls_initializer());
2868-
if (auto* alloc_rec = orig_instance->AsAllocateRecord()) {
2869-
return alloc_rec->field_names()->definition();
2870-
} else if (auto* alloc_rec = orig_instance->AsAllocateSmallRecord()) {
2871-
if (alloc_rec->has_named_fields()) {
2872-
return alloc_rec->field_names()->definition();
2873-
} else {
2874-
return flow_graph->GetConstant(Object::empty_array());
2875-
}
2876-
} else {
2877-
const AbstractType* type = instance()->Type()->ToAbstractType();
2878-
if (type->IsRecordType()) {
2879-
return flow_graph->GetConstant(
2880-
Array::Handle(RecordType::Cast(*type).field_names()));
2843+
Smi::Handle(RecordType::Cast(*type).shape().AsSmi()));
28812844
}
28822845
}
28832846
break;
@@ -7765,12 +7728,10 @@ void SuspendInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
77657728

77667729
LocationSummary* AllocateRecordInstr::MakeLocationSummary(Zone* zone,
77677730
bool opt) const {
7768-
const intptr_t kNumInputs = 1;
7731+
const intptr_t kNumInputs = 0;
77697732
const intptr_t kNumTemps = 0;
77707733
LocationSummary* locs = new (zone)
77717734
LocationSummary(zone, kNumInputs, kNumTemps, LocationSummary::kCall);
7772-
locs->set_in(0,
7773-
Location::RegisterLocation(AllocateRecordABI::kFieldNamesReg));
77747735
locs->set_out(0, Location::RegisterLocation(AllocateRecordABI::kResultReg));
77757736
return locs;
77767737
}
@@ -7779,8 +7740,8 @@ void AllocateRecordInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
77797740
const Code& stub = Code::ZoneHandle(
77807741
compiler->zone(),
77817742
compiler->isolate_group()->object_store()->allocate_record_stub());
7782-
__ LoadImmediate(AllocateRecordABI::kNumFieldsReg,
7783-
Smi::RawValue(num_fields()));
7743+
__ LoadImmediate(AllocateRecordABI::kShapeReg,
7744+
Smi::RawValue(shape().AsInt()));
77847745
compiler->GenerateStubCall(source(), stub, UntaggedPcDescriptors::kOther,
77857746
locs(), deopt_id(), env());
77867747
}
@@ -7792,35 +7753,25 @@ LocationSummary* AllocateSmallRecordInstr::MakeLocationSummary(Zone* zone,
77927753
const intptr_t kNumTemps = 0;
77937754
LocationSummary* locs = new (zone)
77947755
LocationSummary(zone, kNumInputs, kNumTemps, LocationSummary::kCall);
7795-
if (has_named_fields()) {
7796-
locs->set_in(
7797-
0, Location::RegisterLocation(AllocateSmallRecordABI::kFieldNamesReg));
7798-
locs->set_in(
7799-
1, Location::RegisterLocation(AllocateSmallRecordABI::kValue0Reg));
7800-
locs->set_in(
7801-
2, Location::RegisterLocation(AllocateSmallRecordABI::kValue1Reg));
7802-
if (num_fields() > 2) {
7803-
locs->set_in(
7804-
3, Location::RegisterLocation(AllocateSmallRecordABI::kValue2Reg));
7805-
}
7806-
} else {
7807-
locs->set_in(
7808-
0, Location::RegisterLocation(AllocateSmallRecordABI::kValue0Reg));
7756+
locs->set_in(0,
7757+
Location::RegisterLocation(AllocateSmallRecordABI::kValue0Reg));
7758+
locs->set_in(1,
7759+
Location::RegisterLocation(AllocateSmallRecordABI::kValue1Reg));
7760+
if (num_fields() > 2) {
78097761
locs->set_in(
7810-
1, Location::RegisterLocation(AllocateSmallRecordABI::kValue1Reg));
7811-
if (num_fields() > 2) {
7812-
locs->set_in(
7813-
2, Location::RegisterLocation(AllocateSmallRecordABI::kValue2Reg));
7814-
}
7762+
2, Location::RegisterLocation(AllocateSmallRecordABI::kValue2Reg));
78157763
}
7816-
locs->set_out(0, Location::RegisterLocation(AllocateRecordABI::kResultReg));
7764+
locs->set_out(0,
7765+
Location::RegisterLocation(AllocateSmallRecordABI::kResultReg));
78177766
return locs;
78187767
}
78197768

78207769
void AllocateSmallRecordInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
78217770
auto object_store = compiler->isolate_group()->object_store();
78227771
Code& stub = Code::ZoneHandle(compiler->zone());
7823-
if (has_named_fields()) {
7772+
if (shape().HasNamedFields()) {
7773+
__ LoadImmediate(AllocateSmallRecordABI::kShapeReg,
7774+
Smi::RawValue(shape().AsInt()));
78247775
switch (num_fields()) {
78257776
case 2:
78267777
stub = object_store->allocate_record2_named_stub();

0 commit comments

Comments
 (0)