Skip to content

Commit d7c0271

Browse files
mkustermanncommit-bot@chromium.org
mkustermann
authored and
[email protected]
committed
[vm/concurrency] Add missing WeakProperty support to transitive copy
The transitive copy algorithm was missing support for [WeakProperty]s, thereby ensuring that we only copy the values if the keys are reachable. Furthermore we might need to re-hash [Expando]s - since the copied objects start with no identity hash codes. The CL also makes us avoid calling to Dart for each [Expando] separately and instead use a list - just as we do in the re-hashing of maps/sets. We also move the C++ code to invoke rehashing logic into DartLibraryCalls::*. Issue #36097 TEST=vm/dart{,_2}/isolates/fast_object_copy{,2}_test Change-Id: I836745feef8a6d7573faa94e29a19c1eca0c39f2 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/209106 Commit-Queue: Martin Kustermann <[email protected]> Reviewed-by: Alexander Aprelev <[email protected]> Reviewed-by: Ryan Macnak <[email protected]>
1 parent 122cd0c commit d7c0271

15 files changed

+445
-118
lines changed

‎runtime/lib/isolate.cc‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -299,8 +299,12 @@ DEFINE_NATIVE_ENTRY(SendPortImpl_sendAndExitInternal_, 0, 2) {
299299
Object& validated_result = Object::Handle(zone);
300300
const Object& msg_obj = Object::Handle(zone, obj.ptr());
301301
validated_result = ValidateMessageObject(zone, isolate, msg_obj);
302-
// msg_array = [<message>, <object-in-message-to-rehash>]
303-
const Array& msg_array = Array::Handle(zone, Array::New(2));
302+
// msg_array = [
303+
// <message>,
304+
// <collection-lib-objects-to-rehash>,
305+
// <core-lib-objects-to-rehash>,
306+
// ]
307+
const Array& msg_array = Array::Handle(zone, Array::New(3));
304308
msg_array.SetAt(0, msg_obj);
305309
if (validated_result.IsUnhandledException()) {
306310
Exceptions::PropagateError(Error::Cast(validated_result));

‎runtime/tests/vm/dart/isolates/fast_object_copy_test.dart‎

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// for details. All rights reserved. Use of this source code is governed by a
33
// BSD-style license that can be found in the LICENSE file.
44

5-
// VMOptions=
5+
// VMOptions=--no-enable-isolate-groups
66
// VMOptions=--enable-isolate-groups --no-enable-fast-object-copy
77
// VMOptions=--enable-isolate-groups --enable-fast-object-copy
88
// VMOptions=--enable-isolate-groups --no-enable-fast-object-copy --gc-on-foc-slow-path --force-evacuation --verify-store-buffer
@@ -219,6 +219,8 @@ class SendReceiveTest extends SendReceiveTestBase {
219219

220220
await testFastOnly();
221221
await testSlowOnly();
222+
223+
await testWeakProperty();
222224
}
223225

224226
Future testTransferrable() async {
@@ -579,6 +581,69 @@ class SendReceiveTest extends SendReceiveTestBase {
579581
await sendReceive([notAllocatableInTLAB, smallContainer]));
580582
}
581583
}
584+
585+
Future testWeakProperty() async {
586+
final key = Object();
587+
final expando1 = Expando();
588+
final expando2 = Expando();
589+
final expando3 = Expando();
590+
final expando4 = Expando();
591+
expando1[key] = expando2;
592+
expando2[expando2] = expando3;
593+
expando3[expando3] = expando4;
594+
expando4[expando4] = {'foo': 'bar'};
595+
596+
{
597+
final result = await sendReceive([
598+
key,
599+
expando1,
600+
]);
601+
final keyCopy = result[0];
602+
final expando1Copy = result[1] as Expando;
603+
final expando2Copy = expando1Copy[keyCopy] as Expando;
604+
final expando3Copy = expando2Copy[expando2Copy] as Expando;
605+
final expando4Copy = expando3Copy[expando3Copy] as Expando;
606+
Expect.equals('bar', (expando4Copy[expando4Copy] as Map)['foo']);
607+
}
608+
{
609+
final result = await sendReceive([
610+
expando1,
611+
key,
612+
]);
613+
final expando1Copy = result[0] as Expando;
614+
final keyCopy = result[1];
615+
final expando2Copy = expando1Copy[keyCopy] as Expando;
616+
final expando3Copy = expando2Copy[expando2Copy] as Expando;
617+
final expando4Copy = expando3Copy[expando3Copy] as Expando;
618+
Expect.equals('bar', (expando4Copy[expando4Copy] as Map)['foo']);
619+
}
620+
{
621+
final result = await sendReceive([
622+
expando1,
623+
notAllocatableInTLAB,
624+
key,
625+
]);
626+
final expando1Copy = result[0] as Expando;
627+
final keyCopy = result[2];
628+
final expando2Copy = expando1Copy[keyCopy] as Expando;
629+
final expando3Copy = expando2Copy[expando2Copy] as Expando;
630+
final expando4Copy = expando3Copy[expando3Copy] as Expando;
631+
Expect.equals('bar', (expando4Copy[expando4Copy] as Map)['foo']);
632+
}
633+
{
634+
final result = await sendReceive([
635+
key,
636+
notAllocatableInTLAB,
637+
expando1,
638+
]);
639+
final keyCopy = result[0];
640+
final expando1Copy = result[2] as Expando;
641+
final expando2Copy = expando1Copy[keyCopy] as Expando;
642+
final expando3Copy = expando2Copy[expando2Copy] as Expando;
643+
final expando4Copy = expando3Copy[expando3Copy] as Expando;
644+
Expect.equals('bar', (expando4Copy[expando4Copy] as Map)['foo']);
645+
}
646+
}
582647
}
583648

584649
main() async {

‎runtime/tests/vm/dart_2/isolates/fast_object_copy_test.dart‎

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
// @dart = 2.9
66

7-
// VMOptions=
7+
// VMOptions=--no-enable-isolate-groups
88
// VMOptions=--enable-isolate-groups --no-enable-fast-object-copy
99
// VMOptions=--enable-isolate-groups --enable-fast-object-copy
1010
// VMOptions=--enable-isolate-groups --no-enable-fast-object-copy --gc-on-foc-slow-path --force-evacuation --verify-store-buffer
@@ -221,6 +221,8 @@ class SendReceiveTest extends SendReceiveTestBase {
221221

222222
await testFastOnly();
223223
await testSlowOnly();
224+
225+
await testWeakProperty();
224226
}
225227

226228
Future testTransferrable() async {
@@ -581,6 +583,69 @@ class SendReceiveTest extends SendReceiveTestBase {
581583
await sendReceive([notAllocatableInTLAB, smallContainer]));
582584
}
583585
}
586+
587+
Future testWeakProperty() async {
588+
final key = Object();
589+
final expando1 = Expando();
590+
final expando2 = Expando();
591+
final expando3 = Expando();
592+
final expando4 = Expando();
593+
expando1[key] = expando2;
594+
expando2[expando2] = expando3;
595+
expando3[expando3] = expando4;
596+
expando4[expando4] = {'foo': 'bar'};
597+
598+
{
599+
final result = await sendReceive([
600+
key,
601+
expando1,
602+
]);
603+
final keyCopy = result[0];
604+
final expando1Copy = result[1] as Expando;
605+
final expando2Copy = expando1Copy[keyCopy] as Expando;
606+
final expando3Copy = expando2Copy[expando2Copy] as Expando;
607+
final expando4Copy = expando3Copy[expando3Copy] as Expando;
608+
Expect.equals('bar', (expando4Copy[expando4Copy] as Map)['foo']);
609+
}
610+
{
611+
final result = await sendReceive([
612+
expando1,
613+
key,
614+
]);
615+
final expando1Copy = result[0] as Expando;
616+
final keyCopy = result[1];
617+
final expando2Copy = expando1Copy[keyCopy] as Expando;
618+
final expando3Copy = expando2Copy[expando2Copy] as Expando;
619+
final expando4Copy = expando3Copy[expando3Copy] as Expando;
620+
Expect.equals('bar', (expando4Copy[expando4Copy] as Map)['foo']);
621+
}
622+
{
623+
final result = await sendReceive([
624+
expando1,
625+
notAllocatableInTLAB,
626+
key,
627+
]);
628+
final expando1Copy = result[0] as Expando;
629+
final keyCopy = result[2];
630+
final expando2Copy = expando1Copy[keyCopy] as Expando;
631+
final expando3Copy = expando2Copy[expando2Copy] as Expando;
632+
final expando4Copy = expando3Copy[expando3Copy] as Expando;
633+
Expect.equals('bar', (expando4Copy[expando4Copy] as Map)['foo']);
634+
}
635+
{
636+
final result = await sendReceive([
637+
key,
638+
notAllocatableInTLAB,
639+
expando1,
640+
]);
641+
final keyCopy = result[0];
642+
final expando1Copy = result[2] as Expando;
643+
final expando2Copy = expando1Copy[keyCopy] as Expando;
644+
final expando3Copy = expando2Copy[expando2Copy] as Expando;
645+
final expando4Copy = expando3Copy[expando3Copy] as Expando;
646+
Expect.equals('bar', (expando4Copy[expando4Copy] as Map)['foo']);
647+
}
648+
}
584649
}
585650

586651
main() async {

‎runtime/vm/dart_entry.cc‎

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -786,24 +786,36 @@ ObjectPtr DartLibraryCalls::EnsureScheduleImmediate() {
786786
return result.ptr();
787787
}
788788

789-
ObjectPtr DartLibraryCalls::RehashObjects(
790-
Thread* thread,
791-
const Object& array_or_growable_array) {
789+
static ObjectPtr RehashObjects(Zone* zone,
790+
const Library& library,
791+
const Object& array_or_growable_array) {
792792
ASSERT(array_or_growable_array.IsArray() ||
793793
array_or_growable_array.IsGrowableObjectArray());
794-
795-
auto zone = thread->zone();
796-
const Library& collections_lib =
797-
Library::Handle(zone, Library::CollectionLibrary());
798-
const Function& rehashing_function = Function::Handle(
799-
zone,
800-
collections_lib.LookupFunctionAllowPrivate(Symbols::_rehashObjects()));
794+
const auto& rehashing_function = Function::Handle(
795+
zone, library.LookupFunctionAllowPrivate(Symbols::_rehashObjects()));
801796
ASSERT(!rehashing_function.IsNull());
802797

803-
const Array& arguments = Array::Handle(zone, Array::New(1));
798+
const auto& arguments = Array::Handle(zone, Array::New(1));
804799
arguments.SetAt(0, array_or_growable_array);
805800

806801
return DartEntry::InvokeFunction(rehashing_function, arguments);
807802
}
808803

804+
ObjectPtr DartLibraryCalls::RehashObjectsInDartCollection(
805+
Thread* thread,
806+
const Object& array_or_growable_array) {
807+
auto zone = thread->zone();
808+
const auto& collections_lib =
809+
Library::Handle(zone, Library::CollectionLibrary());
810+
return RehashObjects(zone, collections_lib, array_or_growable_array);
811+
}
812+
813+
ObjectPtr DartLibraryCalls::RehashObjectsInDartCore(
814+
Thread* thread,
815+
const Object& array_or_growable_array) {
816+
auto zone = thread->zone();
817+
const auto& core_lib = Library::Handle(zone, Library::CoreLibrary());
818+
return RehashObjects(zone, core_lib, array_or_growable_array);
819+
}
820+
809821
} // namespace dart

‎runtime/vm/dart_entry.h‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -306,9 +306,15 @@ class DartLibraryCalls : public AllStatic {
306306
// Returns null on success, an ErrorPtr on failure.
307307
static ObjectPtr EnsureScheduleImmediate();
308308

309-
// Runs the `_rehashObjects()` function.
310-
static ObjectPtr RehashObjects(Thread* thread,
311-
const Object& array_or_growable_array);
309+
// Runs the `_rehashObjects()` function in `dart:collection`.
310+
static ObjectPtr RehashObjectsInDartCollection(
311+
Thread* thread,
312+
const Object& array_or_growable_array);
313+
314+
// Runs the `_rehashObjects()` function in `dart:core`.
315+
static ObjectPtr RehashObjectsInDartCore(
316+
Thread* thread,
317+
const Object& array_or_growable_array);
312318
};
313319

314320
} // namespace dart

‎runtime/vm/isolate.cc‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1329,14 +1329,29 @@ MessageHandler::MessageStatus IsolateMessageHandler::HandleMessage(
13291329
// Parse the message.
13301330
Object& msg_obj = Object::Handle(zone);
13311331
if (message->IsPersistentHandle()) {
1332-
// msg_array = [<message>, <object-in-message-to-rehash>]
1332+
// msg_array = [
1333+
// <message>,
1334+
// <collection-lib-objects-to-rehash>,
1335+
// <core-lib-objects-to-rehash>,
1336+
// ]
13331337
const auto& msg_array = Array::Handle(
13341338
zone, Array::RawCast(message->persistent_handle()->ptr()));
1339+
ASSERT(msg_array.Length() == 3);
13351340
msg_obj = msg_array.At(0);
13361341
if (msg_array.At(1) != Object::null()) {
13371342
const auto& objects_to_rehash = Object::Handle(zone, msg_array.At(1));
1338-
const auto& result = Object::Handle(
1339-
zone, DartLibraryCalls::RehashObjects(thread, objects_to_rehash));
1343+
auto& result = Object::Handle(zone);
1344+
result = DartLibraryCalls::RehashObjectsInDartCollection(
1345+
thread, objects_to_rehash);
1346+
if (result.ptr() != Object::null()) {
1347+
msg_obj = result.ptr();
1348+
}
1349+
}
1350+
if (msg_array.At(2) != Object::null()) {
1351+
const auto& objects_to_rehash = Object::Handle(zone, msg_array.At(2));
1352+
auto& result = Object::Handle(zone);
1353+
result =
1354+
DartLibraryCalls::RehashObjectsInDartCore(thread, objects_to_rehash);
13401355
if (result.ptr() != Object::null()) {
13411356
msg_obj = result.ptr();
13421357
}

‎runtime/vm/message_snapshot.cc‎

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -483,7 +483,7 @@ ObjectPtr MessageDeserializationCluster::PostLoadLinkedHash(
483483
Array& maps = Array::Handle(d->zone(), d->refs());
484484
maps = maps.Slice(start_index_, stop_index_ - start_index_,
485485
/*with_type_argument=*/false);
486-
return DartLibraryCalls::RehashObjects(d->thread(), maps);
486+
return DartLibraryCalls::RehashObjectsInDartCollection(d->thread(), maps);
487487
}
488488

489489
class ClassMessageSerializationCluster : public MessageSerializationCluster {
@@ -916,18 +916,14 @@ class InstanceMessageDeserializationCluster
916916
}
917917

918918
if (cls_.ptr() == d->isolate_group()->object_store()->expando_class()) {
919-
Instance& instance = Instance::Handle(d->zone());
920-
const String& selector = Library::PrivateCoreLibName(Symbols::_rehash());
921-
Array& args = Array::Handle(d->zone(), Array::New(1));
922-
for (intptr_t i = start_index_; i < stop_index_; i++) {
919+
const auto& expandos =
920+
Array::Handle(d->zone(), Array::New(stop_index_ - start_index_));
921+
auto& instance = Instance::Handle(d->zone());
922+
for (intptr_t i = start_index_, j = 0; i < stop_index_; i++, j++) {
923923
instance ^= d->Ref(i);
924-
args.SetAt(0, instance);
925-
ObjectPtr error = instance.Invoke(selector, args, Object::empty_array(),
926-
false, false);
927-
if (error != Object::null()) {
928-
return error;
929-
}
924+
expandos.SetAt(j, instance);
930925
}
926+
return DartLibraryCalls::RehashObjectsInDartCore(d->thread(), expandos);
931927
}
932928
return nullptr;
933929
}

0 commit comments

Comments
 (0)