Skip to content

Commit

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

This reverts commit 85e396a as it broke internal app running on android arm in release mode.

Change-Id: Iaff0cf3c1ef859e35b4eaeb3ac8243ce3c6737b8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/132168
Reviewed-by: Alexander Aprelev <aam@google.com>
Commit-Queue: Alexander Aprelev <aam@google.com>
  • Loading branch information
@aam
aam authored and commit-bot@chromium.org committed Jan 17, 2020
1 parent 746d3fb commit c8f8c11
Show file tree
Hide file tree
Showing 40 changed files with 434 additions and 910 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'], 'static fields table');
expect(result['elements'].length, equals(1));
expect(result['elements'][0]['value']['type'], equals('@Instance'));
expect(result['gcRootType'], 'user global');
expect(result['elements'].length, equals(2));
expect(result['elements'][1]['value']['name'], equals('globalObject'));
},

// 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'], 'static fields table');
expect(result['elements'].length, equals(2));
expect(result['elements'][0]['value']['type'], equals('@Instance'));
expect(result['gcRootType'], 'user global');
expect(result['elements'].length, equals(3));
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'], 'static fields table');
expect(result['elements'].length, equals(2));
expect(result['gcRootType'], 'user global');
expect(result['elements'].length, equals(3));
expect(result['elements'][1]['parentField'], equals('y'));
expect(result['elements'][1]['value']['type'], equals('@Instance'));
expect(result['elements'][2]['value']['name'], equals('globalObject'));
},

(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'], 'static fields table');
expect(result['elements'].length, equals(2));
expect(result['gcRootType'], 'user global');
expect(result['elements'].length, equals(3));
expect(result['elements'][1]['parentListIndex'], equals(12));
expect(result['elements'][1]['value']['type'], equals('@Instance'));
expect(result['elements'][2]['value']['name'], equals('globalList'));
},

(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'], 'static fields table');
expect(result['elements'].length, equals(2));
expect(result['gcRootType'], 'user global');
expect(result['elements'].length, equals(3));
expect(
result['elements'][1]['parentMapKey']['valueAsString'], equals('key'));
expect(result['elements'][1]['value']['type'], equals('@Instance'));
expect(result['elements'][2]['value']['name'], equals('globalMap1'));
},

(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(2));
expect(result['elements'].length, equals(3));
expect(result['elements'][1]['parentMapKey']['class']['name'],
equals('_TestClass'));
expect(result['elements'][1]['value']['type'], equals('@Instance'));
expect(result['elements'][2]['value']['name'], equals('globalMap2'));
},

// object store
Expand Down
6 changes: 4 additions & 2 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,9 +46,10 @@ var tests = <IsolateTest>[
};
var result = await isolate.invokeRpcNoUpgrade('getRetainingPath', params);
expect(result['type'], equals('RetainingPath'));
expect(result['elements'].length, equals(1));
expect(result['elements'].length, equals(2));
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 @@ -61,8 +62,9 @@ var tests = <IsolateTest>[
};
var result = await isolate.invokeRpcNoUpgrade('getRetainingPath', params);
expect(result['type'], equals('RetainingPath'));
expect(result['elements'].length, equals(1));
expect(result['elements'].length, equals(2));
expect(result['elements'][0]['value']['class']['name'], equals('_Closure'));
expect(result['elements'][1]['value']['name'], equals('fn'));
}
];

Expand Down
2 changes: 2 additions & 0 deletions runtime/observatory/tests/service/inbound_references_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ 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: 34 additions & 59 deletions runtime/vm/clustered_snapshot.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -913,11 +913,26 @@ 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 values
// Write out the saved initial value
s->Push(field->ptr()->saved_initial_value_);
}
if (kind != Snapshot::kFullAOT) {
Expand All @@ -927,22 +942,6 @@ 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 @@ -965,6 +964,22 @@ 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 @@ -989,27 +1004,6 @@ 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 @@ -1051,17 +1045,6 @@ 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 @@ -4361,7 +4344,6 @@ 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 @@ -4816,7 +4798,6 @@ 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 @@ -5033,8 +5014,7 @@ Deserializer::Deserializer(Thread* thread,
image_reader_(NULL),
refs_(NULL),
next_ref_index_(1),
clusters_(NULL),
field_table_(thread->isolate()->field_table()) {
clusters_(NULL) {
if (Snapshot::IncludesCode(kind)) {
ASSERT(instructions_buffer != NULL);
ASSERT(data_buffer != NULL);
Expand Down Expand Up @@ -5302,14 +5282,9 @@ 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: 0 additions & 6 deletions runtime/vm/clustered_snapshot.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -216,9 +216,6 @@ 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 @@ -375,7 +372,6 @@ 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 @@ -560,7 +556,6 @@ 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 @@ -590,7 +585,6 @@ 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 c8f8c11

Please sign in to comment.