Skip to content

Commit

Permalink
[vm/fieldtable] Move current field values out of Field object into se…
Browse files Browse the repository at this point in the history 
…parate table.

This is to support sharing of Field objects between isolate group's isolates.

Each isolate will have it's own field table with static instances/values.
field_table is stored on isolate, copied over to thread for easier access.

This also removes the write barrier from static field assignments; the field table is a GC root.

Bug: dart-lang/sdk#37835
Bug: dart-lang/sdk#36097
Change-Id: I232a86f059ac4cacc3e9f54bcc31d1d0524f9496
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/127582
Commit-Queue: Alexander Aprelev <aam@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
  • Loading branch information
@aam
aam authored and commit-bot@chromium.org committed Jan 6, 2020
1 parent 033422b commit 85e396a
Show file tree
Hide file tree
Showing 40 changed files with 910 additions and 434 deletions.
34 changes: 17 additions & 17 deletions runtime/observatory/tests/service/get_retaining_path_rpc_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,9 @@ var tests = <IsolateTest>[
'limit': 100,
};
var result = await isolate.invokeRpcNoUpgrade('getRetainingPath', params);
expect(result['gcRootType'], 'user global');
expect(result['elements'].length, equals(2));
expect(result['elements'][1]['value']['name'], equals('globalObject'));
expect(result['gcRootType'], 'static fields table');
expect(result['elements'].length, equals(1));
expect(result['elements'][0]['value']['type'], equals('@Instance'));
},

// missing limit.
Expand Down Expand Up @@ -81,10 +81,10 @@ var tests = <IsolateTest>[
};
var result = await isolate.invokeRpcNoUpgrade('getRetainingPath', params);
expect(result['type'], equals('RetainingPath'));
expect(result['gcRootType'], 'user global');
expect(result['elements'].length, equals(3));
expect(result['gcRootType'], 'static fields table');
expect(result['elements'].length, equals(2));
expect(result['elements'][0]['value']['type'], equals('@Instance'));
expect(result['elements'][1]['parentField'], equals('x'));
expect(result['elements'][2]['value']['name'], equals('globalObject'));
},

(Isolate isolate) async {
Expand All @@ -96,10 +96,10 @@ var tests = <IsolateTest>[
};
var result = await isolate.invokeRpcNoUpgrade('getRetainingPath', params);
expect(result['type'], equals('RetainingPath'));
expect(result['gcRootType'], 'user global');
expect(result['elements'].length, equals(3));
expect(result['gcRootType'], 'static fields table');
expect(result['elements'].length, equals(2));
expect(result['elements'][1]['parentField'], equals('y'));
expect(result['elements'][2]['value']['name'], equals('globalObject'));
expect(result['elements'][1]['value']['type'], equals('@Instance'));
},

(Isolate isolate) async {
Expand All @@ -111,10 +111,10 @@ var tests = <IsolateTest>[
};
var result = await isolate.invokeRpcNoUpgrade('getRetainingPath', params);
expect(result['type'], equals('RetainingPath'));
expect(result['gcRootType'], 'user global');
expect(result['elements'].length, equals(3));
expect(result['gcRootType'], 'static fields table');
expect(result['elements'].length, equals(2));
expect(result['elements'][1]['parentListIndex'], equals(12));
expect(result['elements'][2]['value']['name'], equals('globalList'));
expect(result['elements'][1]['value']['type'], equals('@Instance'));
},

(Isolate isolate) async {
Expand All @@ -126,11 +126,11 @@ var tests = <IsolateTest>[
};
var result = await isolate.invokeRpcNoUpgrade('getRetainingPath', params);
expect(result['type'], equals('RetainingPath'));
expect(result['gcRootType'], 'user global');
expect(result['elements'].length, equals(3));
expect(result['gcRootType'], 'static fields table');
expect(result['elements'].length, equals(2));
expect(
result['elements'][1]['parentMapKey']['valueAsString'], equals('key'));
expect(result['elements'][2]['value']['name'], equals('globalMap1'));
expect(result['elements'][1]['value']['type'], equals('@Instance'));
},

(Isolate isolate) async {
Expand All @@ -142,10 +142,10 @@ var tests = <IsolateTest>[
};
var result = await isolate.invokeRpcNoUpgrade('getRetainingPath', params);
expect(result['type'], equals('RetainingPath'));
expect(result['elements'].length, equals(3));
expect(result['elements'].length, equals(2));
expect(result['elements'][1]['parentMapKey']['class']['name'],
equals('_TestClass'));
expect(result['elements'][2]['value']['name'], equals('globalMap2'));
expect(result['elements'][1]['value']['type'], equals('@Instance'));
},

// object store
Expand Down
6 changes: 2 additions & 4 deletions runtime/observatory/tests/service/get_user_level_retaining_path_rpc_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,9 @@ var tests = <IsolateTest>[
};
var result = await isolate.invokeRpcNoUpgrade('getRetainingPath', params);
expect(result['type'], equals('RetainingPath'));
expect(result['elements'].length, equals(2));
expect(result['elements'].length, equals(1));
expect(
result['elements'][0]['value']['class']['name'], equals('_TestConst'));
expect(result['elements'][1]['value']['name'], equals('x'));
},

// Expect a simple path through variable fn instead of long path filled
Expand All @@ -62,9 +61,8 @@ var tests = <IsolateTest>[
};
var result = await isolate.invokeRpcNoUpgrade('getRetainingPath', params);
expect(result['type'], equals('RetainingPath'));
expect(result['elements'].length, equals(2));
expect(result['elements'].length, equals(1));
expect(result['elements'][0]['value']['class']['name'], equals('_Closure'));
expect(result['elements'][1]['value']['name'], equals('fn'));
}
];

Expand Down
2 changes: 0 additions & 2 deletions runtime/observatory/tests/service/inbound_references_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,6 @@ var tests = <IsolateTest>[
r['source'].clazz.name == 'Node');
hasReferenceSuchThat(
(r) => r['parentListIndex'] == 1 && r['source'].isList);
hasReferenceSuchThat(
(r) => r['source'] is Field && r['source'].name == 'e');
}
];

Expand Down
93 changes: 59 additions & 34 deletions runtime/vm/clustered_snapshot.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -913,26 +913,11 @@ class FieldSerializationCluster : public SerializationCluster {
s->Push(field->ptr()->name_);
s->Push(field->ptr()->owner_);
s->Push(field->ptr()->type_);
// Write out the initial static value or field offset.
if (Field::StaticBit::decode(field->ptr()->kind_bits_)) {
if (kind == Snapshot::kFullAOT) {
// For precompiled static fields, the value was already reset and
// initializer_ now contains a Function.
s->Push(field->ptr()->value_.static_value_);
} else if (Field::ConstBit::decode(field->ptr()->kind_bits_)) {
// Do not reset const fields.
s->Push(field->ptr()->value_.static_value_);
} else {
// Otherwise, for static fields we write out the initial static value.
s->Push(field->ptr()->saved_initial_value_);
}
} else {
s->Push(field->ptr()->value_.offset_);
}
// Write out the initializer function
s->Push(field->ptr()->initializer_function_);

if (kind != Snapshot::kFullAOT) {
// Write out the saved initial value
// Write out the saved initial values
s->Push(field->ptr()->saved_initial_value_);
}
if (kind != Snapshot::kFullAOT) {
Expand All @@ -942,6 +927,22 @@ class FieldSerializationCluster : public SerializationCluster {
if (kind == Snapshot::kFullJIT) {
s->Push(field->ptr()->dependent_code_);
}
// Write out either static value, initial value or field offset.
if (Field::StaticBit::decode(field->ptr()->kind_bits_)) {
if (
// For precompiled static fields, the value was already reset and
// initializer_ now contains a Function.
kind == Snapshot::kFullAOT ||
// Do not reset const fields.
Field::ConstBit::decode(field->ptr()->kind_bits_)) {
s->Push(s->field_table()->At(field->ptr()->offset_or_field_id_));
} else {
// Otherwise, for static fields we write out the initial static value.
s->Push(field->ptr()->saved_initial_value_);
}
} else {
s->Push(Smi::New(field->ptr()->offset_or_field_id_));
}
}

void WriteAlloc(Serializer* s) {
Expand All @@ -964,22 +965,6 @@ class FieldSerializationCluster : public SerializationCluster {
WriteField(field, name_);
WriteField(field, owner_);
WriteField(field, type_);
// Write out the initial static value or field offset.
if (Field::StaticBit::decode(field->ptr()->kind_bits_)) {
if (kind == Snapshot::kFullAOT) {
// For precompiled static fields, the value was already reset and
// initializer_ now contains a Function.
WriteField(field, value_.static_value_);
} else if (Field::ConstBit::decode(field->ptr()->kind_bits_)) {
// Do not reset const fields.
WriteField(field, value_.static_value_);
} else {
// Otherwise, for static fields we write out the initial static value.
WriteField(field, saved_initial_value_);
}
} else {
WriteField(field, value_.offset_);
}
// Write out the initializer function and initial value if not in AOT.
WriteField(field, initializer_function_);
if (kind != Snapshot::kFullAOT) {
Expand All @@ -1004,6 +989,27 @@ class FieldSerializationCluster : public SerializationCluster {
#endif
}
s->Write<uint16_t>(field->ptr()->kind_bits_);

// Write out the initial static value or field offset.
if (Field::StaticBit::decode(field->ptr()->kind_bits_)) {
if (
// For precompiled static fields, the value was already reset and
// initializer_ now contains a Function.
// WriteField(field, value_.static_value_);
kind == Snapshot::kFullAOT ||
// Do not reset const fields.
Field::ConstBit::decode(field->ptr()->kind_bits_)) {
WriteFieldValue(
"static value",
s->field_table()->At(field->ptr()->offset_or_field_id_));
} else {
// Otherwise, for static fields we write out the initial static value.
WriteFieldValue("static value", field->ptr()->saved_initial_value_);
}
s->WriteUnsigned(field->ptr()->offset_or_field_id_);
} else {
WriteFieldValue("offset", Smi::New(field->ptr()->offset_or_field_id_));
}
}
}

Expand Down Expand Up @@ -1045,6 +1051,17 @@ class FieldDeserializationCluster : public DeserializationCluster {
#endif
}
field->ptr()->kind_bits_ = d->Read<uint16_t>();

RawObject* value_or_offset = d->ReadRef();
if (Field::StaticBit::decode(field->ptr()->kind_bits_)) {
intptr_t field_id = d->ReadUnsigned();
d->field_table()->SetAt(
field_id, reinterpret_cast<RawInstance*>(value_or_offset));
field->ptr()->offset_or_field_id_ = field_id;
} else {
field->ptr()->offset_or_field_id_ =
Smi::Value(Smi::RawCast(value_or_offset));
}
}
}

Expand Down Expand Up @@ -4342,6 +4359,7 @@ Serializer::Serializer(Thread* thread,
num_base_objects_(0),
num_written_objects_(0),
next_ref_index_(1),
field_table_(thread->isolate()->field_table()),
vm_(vm),
profile_writer_(profile_writer)
#if defined(SNAPSHOT_BACKTRACE)
Expand Down Expand Up @@ -4796,6 +4814,7 @@ void Serializer::Serialize() {
WriteUnsigned(num_objects);
WriteUnsigned(num_clusters);
WriteUnsigned(code_order_length);
WriteUnsigned(field_table_->NumFieldIds());

for (intptr_t cid = 1; cid < num_cids_; cid++) {
SerializationCluster* cluster = clusters_by_cid_[cid];
Expand Down Expand Up @@ -5012,7 +5031,8 @@ Deserializer::Deserializer(Thread* thread,
image_reader_(NULL),
refs_(NULL),
next_ref_index_(1),
clusters_(NULL) {
clusters_(NULL),
field_table_(thread->isolate()->field_table()) {
if (Snapshot::IncludesCode(kind)) {
ASSERT(instructions_buffer != NULL);
ASSERT(data_buffer != NULL);
Expand Down Expand Up @@ -5280,9 +5300,14 @@ void Deserializer::Prepare() {
num_objects_ = ReadUnsigned();
num_clusters_ = ReadUnsigned();
code_order_length_ = ReadUnsigned();
const intptr_t field_table_len = ReadUnsigned();

clusters_ = new DeserializationCluster*[num_clusters_];
refs_ = Array::New(num_objects_ + 1, Heap::kOld);
if (field_table_len > 0) {
field_table_->AllocateIndex(field_table_len - 1);
}
ASSERT(field_table_->NumFieldIds() == field_table_len);
}

void Deserializer::Deserialize() {
Expand Down
6 changes: 6 additions & 0 deletions runtime/vm/clustered_snapshot.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,9 @@ class Serializer : public ThreadStackResource {
void WriteVersionAndFeatures(bool is_vm_snapshot);

void Serialize();

FieldTable* field_table() { return field_table_; }

WriteStream* stream() { return &stream_; }
intptr_t bytes_written() { return stream_.bytes_written(); }

Expand Down Expand Up @@ -372,6 +375,7 @@ class Serializer : public ThreadStackResource {
intptr_t num_written_objects_;
intptr_t next_ref_index_;
SmiObjectIdMap smi_ids_;
FieldTable* field_table_;

// True if writing VM snapshot, false for Isolate snapshot.
bool vm_;
Expand Down Expand Up @@ -556,6 +560,7 @@ class Deserializer : public ThreadStackResource {
intptr_t next_index() const { return next_ref_index_; }
Heap* heap() const { return heap_; }
Snapshot::Kind kind() const { return kind_; }
FieldTable* field_table() const { return field_table_; }

// The number of code objects which were relocated during AOT snapshot
// writing.
Expand Down Expand Up @@ -585,6 +590,7 @@ class Deserializer : public ThreadStackResource {
RawArray* refs_;
intptr_t next_ref_index_;
DeserializationCluster** clusters_;
FieldTable* field_table_;
};

#define ReadFromTo(obj, ...) d->ReadFromTo(obj, ##__VA_ARGS__);
Expand Down
Loading

0 comments on commit 85e396a

Please sign in to comment.