[vm] Ensure transitive object copy allows low safepoint latencies
To make responsiveness to safepointing requests better when a thread
does a transitive object copy we add safepoint checkins.
In the fast object copy implementation - which runs inside
NoSafepointScope - we bail out to the slow path if there's a safepoint
requested.
The slow path is safe to GC at any point due to having all objects
inside handles / GC-visible objects.
TEST=Existing test suite.
Change-Id: Ie2c37e2f618506ab62a592aa9ff9ab266a02b64b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/245166
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
diff --git a/runtime/tests/vm/dart/isolates/fast_object_copy_test.dart b/runtime/tests/vm/dart/isolates/fast_object_copy_test.dart
index 7dc9690..22bf3ac 100644
--- a/runtime/tests/vm/dart/isolates/fast_object_copy_test.dart
+++ b/runtime/tests/vm/dart/isolates/fast_object_copy_test.dart
@@ -49,12 +49,20 @@
})(),
];
+Uint8List initializeUint8List(Uint8List l) {
+ for (int i = 0; i < l.length; ++i) {
+ l[i] = i % 256;
+ }
+ return l;
+}
+
final Uint8List largeExternalTypedData =
- File(Platform.resolvedExecutable).readAsBytesSync()..[0] = 42;
-final Uint8List largeInternalTypedData = Uint8List(20 * 1024 * 1024)..[0] = 42;
+ initializeUint8List(File(Platform.resolvedExecutable).readAsBytesSync());
+final Uint8List largeInternalTypedData =
+ initializeUint8List(Uint8List(20 * 1024 * 1024));
final Uint8List smallExternalTypedData =
- File(Platform.script.toFilePath()).readAsBytesSync()..[0] = 21;
+ initializeUint8List(File(Platform.script.toFilePath()).readAsBytesSync());
final Uint8List smallExternalTypedDataView =
Uint8List.view(smallExternalTypedData.buffer, 1, 1);
@@ -319,7 +327,7 @@
for (int i = 0; i < 10; ++i) {
final result = await sendReceive(graph);
final etd = result[1];
- Expect.equals(42, etd[0]);
+ expectGraphsMatch(largeExternalTypedData, etd);
}
}
@@ -331,8 +339,8 @@
];
for (int i = 0; i < 10; ++i) {
final result = await sendReceive(graph);
- final etd = result[1];
- Expect.equals(42, etd[0]);
+ final etd = result[0];
+ expectGraphsMatch(largeExternalTypedData, etd);
}
}
@@ -359,7 +367,7 @@
print('testExternalTypedData5');
for (int i = 0; i < 10; ++i) {
final etd = await sendReceive(largeExternalTypedData);
- Expect.equals(42, etd[0]);
+ expectGraphsMatch(largeExternalTypedData, etd);
}
}
@@ -369,8 +377,8 @@
smallExternalTypedData,
largeExternalTypedData,
]);
- Expect.equals(21, etd[0][0]);
- Expect.equals(42, etd[1][0]);
+ expectGraphsMatch(smallExternalTypedData, etd[0]);
+ expectGraphsMatch(largeExternalTypedData, etd[1]);
}
Future testInternalTypedDataView() async {
diff --git a/runtime/tests/vm/dart_2/isolates/fast_object_copy_test.dart b/runtime/tests/vm/dart_2/isolates/fast_object_copy_test.dart
index fc62573..1e3773d 100644
--- a/runtime/tests/vm/dart_2/isolates/fast_object_copy_test.dart
+++ b/runtime/tests/vm/dart_2/isolates/fast_object_copy_test.dart
@@ -51,12 +51,20 @@
})(),
];
+Uint8List initializeUint8List(Uint8List l) {
+ for (int i = 0; i < l.length; ++i) {
+ l[i] = i % 256;
+ }
+ return l;
+}
+
final Uint8List largeExternalTypedData =
- File(Platform.resolvedExecutable).readAsBytesSync()..[0] = 42;
-final Uint8List largeInternalTypedData = Uint8List(20 * 1024 * 1024)..[0] = 42;
+ initializeUint8List(File(Platform.resolvedExecutable).readAsBytesSync());
+final Uint8List largeInternalTypedData =
+ initializeUint8List(Uint8List(20 * 1024 * 1024));
final Uint8List smallExternalTypedData =
- File(Platform.script.toFilePath()).readAsBytesSync()..[0] = 21;
+ initializeUint8List(File(Platform.script.toFilePath()).readAsBytesSync());
final Uint8List smallExternalTypedDataView =
Uint8List.view(smallExternalTypedData.buffer, 1, 1);
@@ -321,7 +329,7 @@
for (int i = 0; i < 10; ++i) {
final result = await sendReceive(graph);
final etd = result[1];
- Expect.equals(42, etd[0]);
+ expectGraphsMatch(largeExternalTypedData, etd);
}
}
@@ -333,8 +341,8 @@
];
for (int i = 0; i < 10; ++i) {
final result = await sendReceive(graph);
- final etd = result[1];
- Expect.equals(42, etd[0]);
+ final etd = result[0];
+ expectGraphsMatch(largeExternalTypedData, etd);
}
}
@@ -361,7 +369,7 @@
print('testExternalTypedData5');
for (int i = 0; i < 10; ++i) {
final etd = await sendReceive(largeExternalTypedData);
- Expect.equals(42, etd[0]);
+ expectGraphsMatch(largeExternalTypedData, etd);
}
}
@@ -371,8 +379,8 @@
smallExternalTypedData,
largeExternalTypedData,
]);
- Expect.equals(21, etd[0][0]);
- Expect.equals(42, etd[1][0]);
+ expectGraphsMatch(smallExternalTypedData, etd[0]);
+ expectGraphsMatch(largeExternalTypedData, etd[1]);
}
Future testInternalTypedDataView() async {
diff --git a/runtime/vm/object_graph_copy.cc b/runtime/vm/object_graph_copy.cc
index c49af9a..86d3c8e 100644
--- a/runtime/vm/object_graph_copy.cc
+++ b/runtime/vm/object_graph_copy.cc
@@ -293,6 +293,46 @@
raw_to->data_ = buffer;
}
+template <typename T>
+void CopyTypedDataBaseWithSafepointChecks(Thread* thread,
+ const T& from,
+ const T& to,
+ intptr_t length) {
+ constexpr intptr_t kChunkSize = 100 * 1024;
+
+ const intptr_t chunks = length / kChunkSize;
+ const intptr_t remainder = length % kChunkSize;
+
+ // Notice we re-load the data pointer, since T may be TypedData in which case
+ // the interior pointer may change after checking into safepoints.
+ for (intptr_t i = 0; i < chunks; ++i) {
+ memmove(to.ptr().untag()->data_ + i * kChunkSize,
+ from.ptr().untag()->data_ + i * kChunkSize, kChunkSize);
+
+ thread->CheckForSafepoint();
+ }
+ if (remainder > 0) {
+ memmove(to.ptr().untag()->data_ + chunks * kChunkSize,
+ from.ptr().untag()->data_ + chunks * kChunkSize, remainder);
+ }
+}
+
+void InitializeExternalTypedDataWithSafepointChecks(
+ Thread* thread,
+ intptr_t cid,
+ const ExternalTypedData& from,
+ const ExternalTypedData& to) {
+ const intptr_t length_in_elements = from.Length();
+ const intptr_t length_in_bytes =
+ TypedData::ElementSizeInBytes(cid) * length_in_elements;
+
+ uint8_t* to_data = static_cast<uint8_t*>(malloc(length_in_bytes));
+ to.ptr().untag()->data_ = to_data;
+ to.ptr().untag()->length_ = Smi::New(length_in_elements);
+
+ CopyTypedDataBaseWithSafepointChecks(thread, from, to, length_in_bytes);
+}
+
void InitializeTypedDataView(TypedDataViewPtr obj) {
obj.untag()->typed_data_ = TypedDataBase::null();
obj.untag()->offset_in_bytes_ = 0;
@@ -462,8 +502,10 @@
void AddWeakReference(const WeakReference& from) {
weak_references_.Add(&WeakReference::Handle(from.ptr()));
}
- void AddExternalTypedData(ExternalTypedDataPtr to) {
- external_typed_data_.Add(&ExternalTypedData::Handle(to));
+ const ExternalTypedData& AddExternalTypedData(ExternalTypedDataPtr to) {
+ auto to_handle = &ExternalTypedData::Handle(to);
+ external_typed_data_.Add(to_handle);
+ return *to_handle;
}
void AddObjectToRehash(const Object& to) {
objects_to_rehash_.Add(&Object::Handle(to.ptr()));
@@ -848,6 +890,7 @@
if (Array::UseCardMarkingForAllocation(array_length)) {
for (; offset < end_offset; offset += kCompressedWordSize) {
ForwardCompressedLargeArrayPointer(src, dst, offset);
+ thread_->CheckForSafepoint();
}
} else {
for (; offset < end_offset; offset += kCompressedWordSize) {
@@ -949,9 +992,11 @@
to.untag()->SetCardRememberedBitUnsynchronized();
}
if (IsExternalTypedDataClassId(cid)) {
- InitializeExternalTypedData(cid, ExternalTypedData::RawCast(from.ptr()),
- ExternalTypedData::RawCast(to));
- slow_forward_map_.AddExternalTypedData(ExternalTypedData::RawCast(to));
+ const auto& external_to = slow_forward_map_.AddExternalTypedData(
+ ExternalTypedData::RawCast(to));
+ InitializeExternalTypedDataWithSafepointChecks(
+ thread_, cid, ExternalTypedData::Cast(from), external_to);
+ return external_to.ptr();
} else if (IsTypedDataViewClassId(cid)) {
// We set the views backing store to `null` to satisfy an assertion in
// GCCompactor::VisitTypedDataViewPointers().
@@ -1281,10 +1326,9 @@
#endif
}
- void CopyTypedData(typename Types::TypedData from,
- typename Types::TypedData to) {
- auto raw_from = UntagTypedData(from);
- auto raw_to = UntagTypedData(to);
+ void CopyTypedData(TypedDataPtr from, TypedDataPtr to) {
+ auto raw_from = from.untag();
+ auto raw_to = to.untag();
const intptr_t cid = Types::GetTypedDataPtr(from)->GetClassId();
raw_to->length_ = raw_from->length_;
raw_to->RecomputeDataField();
@@ -1293,6 +1337,17 @@
memmove(raw_to->data_, raw_from->data_, length);
}
+ void CopyTypedData(const TypedData& from, const TypedData& to) {
+ auto raw_from = from.ptr().untag();
+ auto raw_to = to.ptr().untag();
+ const intptr_t cid = Types::GetTypedDataPtr(from)->GetClassId();
+ raw_to->length_ = raw_from->length_;
+ raw_to->RecomputeDataField();
+ const intptr_t length =
+ TypedData::ElementSizeInBytes(cid) * Smi::Value(raw_from->length_);
+ CopyTypedDataBaseWithSafepointChecks(Base::thread_, from, to, length);
+ }
+
void CopyTypedDataView(typename Types::TypedDataView from,
typename Types::TypedDataView to) {
// This will forward & initialize the typed data.
@@ -1457,6 +1512,13 @@
return root_copy;
}
fast_forward_map_.fill_cursor_ += 2;
+
+ // To maintain responsiveness we regularly check whether safepoints are
+ // requested - if so, we bail to slow path which will then checkin.
+ if (thread_->IsSafepointRequested()) {
+ exception_msg_ = kFastAllocationFailed;
+ return root_copy;
+ }
}
// Possibly forward values of [WeakProperty]s if keys became reachable.
@@ -1626,6 +1688,9 @@
if (exception_msg_ != nullptr) {
return Marker();
}
+ // To maintain responsiveness we regularly check whether safepoints are
+ // requested.
+ thread_->CheckForSafepoint();
}
// Possibly forward values of [WeakProperty]s if keys became reachable.
diff --git a/runtime/vm/raw_object.h b/runtime/vm/raw_object.h
index f490865..d590114 100644
--- a/runtime/vm/raw_object.h
+++ b/runtime/vm/raw_object.h
@@ -2896,6 +2896,13 @@
uint8_t* data_;
private:
+ template <typename T>
+ friend void CopyTypedDataBaseWithSafepointChecks(
+ Thread*,
+ const T&,
+ const T&,
+ intptr_t); // Access _data for memmove with safepoint checkins.
+
RAW_HEAP_OBJECT_IMPLEMENTATION(PointerBase);
};
@@ -2918,6 +2925,11 @@
intptr_t,
ExternalTypedDataPtr,
ExternalTypedDataPtr); // initialize fields.
+ friend void InitializeExternalTypedDataWithSafepointChecks(
+ Thread*,
+ intptr_t,
+ const ExternalTypedData&,
+ const ExternalTypedData&); // initialize fields.
RAW_HEAP_OBJECT_IMPLEMENTATION(TypedDataBase);
};
