From d7c0271ac0d37f4c6391e77af3fcf987aaf47cfb Mon Sep 17 00:00:00 2001 From: Martin Kustermann Date: Mon, 9 Aug 2021 23:56:19 +0000 Subject: [PATCH] [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 https://github.com/dart-lang/sdk/issues/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 Reviewed-by: Alexander Aprelev Reviewed-by: Ryan Macnak --- runtime/lib/isolate.cc | 8 +- .../dart/isolates/fast_object_copy_test.dart | 67 +++- .../isolates/fast_object_copy_test.dart | 67 +++- runtime/vm/dart_entry.cc | 34 +- runtime/vm/dart_entry.h | 12 +- runtime/vm/isolate.cc | 21 +- runtime/vm/message_snapshot.cc | 18 +- runtime/vm/object_graph_copy.cc | 296 +++++++++++++----- runtime/vm/raw_object.h | 2 + runtime/vm/symbols.h | 1 - sdk/lib/_internal/vm/lib/expando_patch.dart | 11 +- .../isolate/weak_property_message_1_test.dart | 5 +- .../isolate/weak_property_message_2_test.dart | 8 +- .../isolate/weak_property_message_1_test.dart | 5 +- .../isolate/weak_property_message_2_test.dart | 8 +- 15 files changed, 445 insertions(+), 118 deletions(-) diff --git a/runtime/lib/isolate.cc b/runtime/lib/isolate.cc index 0b72f7f2bac..48cb89095b3 100644 --- a/runtime/lib/isolate.cc +++ b/runtime/lib/isolate.cc @@ -299,8 +299,12 @@ DEFINE_NATIVE_ENTRY(SendPortImpl_sendAndExitInternal_, 0, 2) { Object& validated_result = Object::Handle(zone); const Object& msg_obj = Object::Handle(zone, obj.ptr()); validated_result = ValidateMessageObject(zone, isolate, msg_obj); - // msg_array = [, ] - const Array& msg_array = Array::Handle(zone, Array::New(2)); + // msg_array = [ + // , + // , + // , + // ] + const Array& msg_array = Array::Handle(zone, Array::New(3)); msg_array.SetAt(0, msg_obj); if (validated_result.IsUnhandledException()) { Exceptions::PropagateError(Error::Cast(validated_result)); 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 dda2b0fe564..4f71faea24c 100644 --- a/runtime/tests/vm/dart/isolates/fast_object_copy_test.dart +++ b/runtime/tests/vm/dart/isolates/fast_object_copy_test.dart @@ -2,7 +2,7 @@ // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. -// VMOptions= +// VMOptions=--no-enable-isolate-groups // VMOptions=--enable-isolate-groups --no-enable-fast-object-copy // VMOptions=--enable-isolate-groups --enable-fast-object-copy // 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 { await testFastOnly(); await testSlowOnly(); + + await testWeakProperty(); } Future testTransferrable() async { @@ -579,6 +581,69 @@ class SendReceiveTest extends SendReceiveTestBase { await sendReceive([notAllocatableInTLAB, smallContainer])); } } + + Future testWeakProperty() async { + final key = Object(); + final expando1 = Expando(); + final expando2 = Expando(); + final expando3 = Expando(); + final expando4 = Expando(); + expando1[key] = expando2; + expando2[expando2] = expando3; + expando3[expando3] = expando4; + expando4[expando4] = {'foo': 'bar'}; + + { + final result = await sendReceive([ + key, + expando1, + ]); + final keyCopy = result[0]; + final expando1Copy = result[1] as Expando; + final expando2Copy = expando1Copy[keyCopy] as Expando; + final expando3Copy = expando2Copy[expando2Copy] as Expando; + final expando4Copy = expando3Copy[expando3Copy] as Expando; + Expect.equals('bar', (expando4Copy[expando4Copy] as Map)['foo']); + } + { + final result = await sendReceive([ + expando1, + key, + ]); + final expando1Copy = result[0] as Expando; + final keyCopy = result[1]; + final expando2Copy = expando1Copy[keyCopy] as Expando; + final expando3Copy = expando2Copy[expando2Copy] as Expando; + final expando4Copy = expando3Copy[expando3Copy] as Expando; + Expect.equals('bar', (expando4Copy[expando4Copy] as Map)['foo']); + } + { + final result = await sendReceive([ + expando1, + notAllocatableInTLAB, + key, + ]); + final expando1Copy = result[0] as Expando; + final keyCopy = result[2]; + final expando2Copy = expando1Copy[keyCopy] as Expando; + final expando3Copy = expando2Copy[expando2Copy] as Expando; + final expando4Copy = expando3Copy[expando3Copy] as Expando; + Expect.equals('bar', (expando4Copy[expando4Copy] as Map)['foo']); + } + { + final result = await sendReceive([ + key, + notAllocatableInTLAB, + expando1, + ]); + final keyCopy = result[0]; + final expando1Copy = result[2] as Expando; + final expando2Copy = expando1Copy[keyCopy] as Expando; + final expando3Copy = expando2Copy[expando2Copy] as Expando; + final expando4Copy = expando3Copy[expando3Copy] as Expando; + Expect.equals('bar', (expando4Copy[expando4Copy] as Map)['foo']); + } + } } main() 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 933283f01c9..73b82f47dac 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 @@ -4,7 +4,7 @@ // @dart = 2.9 -// VMOptions= +// VMOptions=--no-enable-isolate-groups // VMOptions=--enable-isolate-groups --no-enable-fast-object-copy // VMOptions=--enable-isolate-groups --enable-fast-object-copy // 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 { await testFastOnly(); await testSlowOnly(); + + await testWeakProperty(); } Future testTransferrable() async { @@ -581,6 +583,69 @@ class SendReceiveTest extends SendReceiveTestBase { await sendReceive([notAllocatableInTLAB, smallContainer])); } } + + Future testWeakProperty() async { + final key = Object(); + final expando1 = Expando(); + final expando2 = Expando(); + final expando3 = Expando(); + final expando4 = Expando(); + expando1[key] = expando2; + expando2[expando2] = expando3; + expando3[expando3] = expando4; + expando4[expando4] = {'foo': 'bar'}; + + { + final result = await sendReceive([ + key, + expando1, + ]); + final keyCopy = result[0]; + final expando1Copy = result[1] as Expando; + final expando2Copy = expando1Copy[keyCopy] as Expando; + final expando3Copy = expando2Copy[expando2Copy] as Expando; + final expando4Copy = expando3Copy[expando3Copy] as Expando; + Expect.equals('bar', (expando4Copy[expando4Copy] as Map)['foo']); + } + { + final result = await sendReceive([ + expando1, + key, + ]); + final expando1Copy = result[0] as Expando; + final keyCopy = result[1]; + final expando2Copy = expando1Copy[keyCopy] as Expando; + final expando3Copy = expando2Copy[expando2Copy] as Expando; + final expando4Copy = expando3Copy[expando3Copy] as Expando; + Expect.equals('bar', (expando4Copy[expando4Copy] as Map)['foo']); + } + { + final result = await sendReceive([ + expando1, + notAllocatableInTLAB, + key, + ]); + final expando1Copy = result[0] as Expando; + final keyCopy = result[2]; + final expando2Copy = expando1Copy[keyCopy] as Expando; + final expando3Copy = expando2Copy[expando2Copy] as Expando; + final expando4Copy = expando3Copy[expando3Copy] as Expando; + Expect.equals('bar', (expando4Copy[expando4Copy] as Map)['foo']); + } + { + final result = await sendReceive([ + key, + notAllocatableInTLAB, + expando1, + ]); + final keyCopy = result[0]; + final expando1Copy = result[2] as Expando; + final expando2Copy = expando1Copy[keyCopy] as Expando; + final expando3Copy = expando2Copy[expando2Copy] as Expando; + final expando4Copy = expando3Copy[expando3Copy] as Expando; + Expect.equals('bar', (expando4Copy[expando4Copy] as Map)['foo']); + } + } } main() async { diff --git a/runtime/vm/dart_entry.cc b/runtime/vm/dart_entry.cc index f062909e8bb..8b2adff92d8 100644 --- a/runtime/vm/dart_entry.cc +++ b/runtime/vm/dart_entry.cc @@ -786,24 +786,36 @@ ObjectPtr DartLibraryCalls::EnsureScheduleImmediate() { return result.ptr(); } -ObjectPtr DartLibraryCalls::RehashObjects( - Thread* thread, - const Object& array_or_growable_array) { +static ObjectPtr RehashObjects(Zone* zone, + const Library& library, + const Object& array_or_growable_array) { ASSERT(array_or_growable_array.IsArray() || array_or_growable_array.IsGrowableObjectArray()); - - auto zone = thread->zone(); - const Library& collections_lib = - Library::Handle(zone, Library::CollectionLibrary()); - const Function& rehashing_function = Function::Handle( - zone, - collections_lib.LookupFunctionAllowPrivate(Symbols::_rehashObjects())); + const auto& rehashing_function = Function::Handle( + zone, library.LookupFunctionAllowPrivate(Symbols::_rehashObjects())); ASSERT(!rehashing_function.IsNull()); - const Array& arguments = Array::Handle(zone, Array::New(1)); + const auto& arguments = Array::Handle(zone, Array::New(1)); arguments.SetAt(0, array_or_growable_array); return DartEntry::InvokeFunction(rehashing_function, arguments); } +ObjectPtr DartLibraryCalls::RehashObjectsInDartCollection( + Thread* thread, + const Object& array_or_growable_array) { + auto zone = thread->zone(); + const auto& collections_lib = + Library::Handle(zone, Library::CollectionLibrary()); + return RehashObjects(zone, collections_lib, array_or_growable_array); +} + +ObjectPtr DartLibraryCalls::RehashObjectsInDartCore( + Thread* thread, + const Object& array_or_growable_array) { + auto zone = thread->zone(); + const auto& core_lib = Library::Handle(zone, Library::CoreLibrary()); + return RehashObjects(zone, core_lib, array_or_growable_array); +} + } // namespace dart diff --git a/runtime/vm/dart_entry.h b/runtime/vm/dart_entry.h index 57403378156..c69d5c8b95e 100644 --- a/runtime/vm/dart_entry.h +++ b/runtime/vm/dart_entry.h @@ -306,9 +306,15 @@ class DartLibraryCalls : public AllStatic { // Returns null on success, an ErrorPtr on failure. static ObjectPtr EnsureScheduleImmediate(); - // Runs the `_rehashObjects()` function. - static ObjectPtr RehashObjects(Thread* thread, - const Object& array_or_growable_array); + // Runs the `_rehashObjects()` function in `dart:collection`. + static ObjectPtr RehashObjectsInDartCollection( + Thread* thread, + const Object& array_or_growable_array); + + // Runs the `_rehashObjects()` function in `dart:core`. + static ObjectPtr RehashObjectsInDartCore( + Thread* thread, + const Object& array_or_growable_array); }; } // namespace dart diff --git a/runtime/vm/isolate.cc b/runtime/vm/isolate.cc index b1a73159765..458cc408ba6 100644 --- a/runtime/vm/isolate.cc +++ b/runtime/vm/isolate.cc @@ -1329,14 +1329,29 @@ MessageHandler::MessageStatus IsolateMessageHandler::HandleMessage( // Parse the message. Object& msg_obj = Object::Handle(zone); if (message->IsPersistentHandle()) { - // msg_array = [, ] + // msg_array = [ + // , + // , + // , + // ] const auto& msg_array = Array::Handle( zone, Array::RawCast(message->persistent_handle()->ptr())); + ASSERT(msg_array.Length() == 3); msg_obj = msg_array.At(0); if (msg_array.At(1) != Object::null()) { const auto& objects_to_rehash = Object::Handle(zone, msg_array.At(1)); - const auto& result = Object::Handle( - zone, DartLibraryCalls::RehashObjects(thread, objects_to_rehash)); + auto& result = Object::Handle(zone); + result = DartLibraryCalls::RehashObjectsInDartCollection( + thread, objects_to_rehash); + if (result.ptr() != Object::null()) { + msg_obj = result.ptr(); + } + } + if (msg_array.At(2) != Object::null()) { + const auto& objects_to_rehash = Object::Handle(zone, msg_array.At(2)); + auto& result = Object::Handle(zone); + result = + DartLibraryCalls::RehashObjectsInDartCore(thread, objects_to_rehash); if (result.ptr() != Object::null()) { msg_obj = result.ptr(); } diff --git a/runtime/vm/message_snapshot.cc b/runtime/vm/message_snapshot.cc index b9400ee7bbd..c3ab4ec4389 100644 --- a/runtime/vm/message_snapshot.cc +++ b/runtime/vm/message_snapshot.cc @@ -483,7 +483,7 @@ ObjectPtr MessageDeserializationCluster::PostLoadLinkedHash( Array& maps = Array::Handle(d->zone(), d->refs()); maps = maps.Slice(start_index_, stop_index_ - start_index_, /*with_type_argument=*/false); - return DartLibraryCalls::RehashObjects(d->thread(), maps); + return DartLibraryCalls::RehashObjectsInDartCollection(d->thread(), maps); } class ClassMessageSerializationCluster : public MessageSerializationCluster { @@ -916,18 +916,14 @@ class InstanceMessageDeserializationCluster } if (cls_.ptr() == d->isolate_group()->object_store()->expando_class()) { - Instance& instance = Instance::Handle(d->zone()); - const String& selector = Library::PrivateCoreLibName(Symbols::_rehash()); - Array& args = Array::Handle(d->zone(), Array::New(1)); - for (intptr_t i = start_index_; i < stop_index_; i++) { + const auto& expandos = + Array::Handle(d->zone(), Array::New(stop_index_ - start_index_)); + auto& instance = Instance::Handle(d->zone()); + for (intptr_t i = start_index_, j = 0; i < stop_index_; i++, j++) { instance ^= d->Ref(i); - args.SetAt(0, instance); - ObjectPtr error = instance.Invoke(selector, args, Object::empty_array(), - false, false); - if (error != Object::null()) { - return error; - } + expandos.SetAt(j, instance); } + return DartLibraryCalls::RehashObjectsInDartCore(d->thread(), expandos); } return nullptr; } diff --git a/runtime/vm/object_graph_copy.cc b/runtime/vm/object_graph_copy.cc index 7b9189f6304..ea1edf5a32d 100644 --- a/runtime/vm/object_graph_copy.cc +++ b/runtime/vm/object_graph_copy.cc @@ -3,11 +3,13 @@ // BSD-style license that can be found in the LICENSE file. #include "vm/object_graph_copy.h" + #include "vm/dart_api_state.h" #include "vm/flags.h" #include "vm/heap/weak_table.h" #include "vm/longjump.h" #include "vm/object.h" +#include "vm/object_store.h" #include "vm/snapshot.h" #include "vm/symbols.h" @@ -79,7 +81,6 @@ V(UnlinkedCall) \ V(UnwindError) \ V(UserTag) \ - V(WeakProperty) \ V(WeakSerializationReference) namespace dart { @@ -353,7 +354,8 @@ class FastForwardMap : public ForwardMapBase { : ForwardMapBase(thread), raw_from_to_(thread->zone(), 20), raw_transferables_from_to_(thread->zone(), 0), - raw_objects_to_rehash_(thread->zone(), 0) { + raw_objects_to_rehash_(thread->zone(), 0), + raw_expandos_to_rehash_(thread->zone(), 0) { raw_from_to_.Resize(2); raw_from_to_[0] = Object::null(); raw_from_to_[1] = Object::null(); @@ -381,11 +383,13 @@ class FastForwardMap : public ForwardMapBase { raw_transferables_from_to_.Add(from); raw_transferables_from_to_.Add(to); } + void AddWeakProperty(WeakPropertyPtr from) { raw_weak_properties_.Add(from); } void AddExternalTypedData(ExternalTypedDataPtr to) { raw_external_typed_data_to_.Add(to); } void AddObjectToRehash(ObjectPtr to) { raw_objects_to_rehash_.Add(to); } + void AddExpandoToRehash(ObjectPtr to) { raw_expandos_to_rehash_.Add(to); } private: friend class FastObjectCopy; @@ -395,6 +399,8 @@ class FastForwardMap : public ForwardMapBase { GrowableArray raw_transferables_from_to_; GrowableArray raw_external_typed_data_to_; GrowableArray raw_objects_to_rehash_; + GrowableArray raw_expandos_to_rehash_; + GrowableArray raw_weak_properties_; intptr_t fill_cursor_ = 0; DISALLOW_COPY_AND_ASSIGN(FastForwardMap); @@ -432,14 +438,18 @@ class SlowForwardMap : public ForwardMapBase { transferables_from_to_.Add(&TransferableTypedData::Handle(from.ptr())); transferables_from_to_.Add(&TransferableTypedData::Handle(to.ptr())); } - + void AddWeakProperty(const WeakProperty& from) { + weak_properties_.Add(&WeakProperty::Handle(from.ptr())); + } void AddExternalTypedData(ExternalTypedDataPtr to) { external_typed_data_.Add(&ExternalTypedData::Handle(to)); } - void AddObjectToRehash(const Object& to) { objects_to_rehash_.Add(&Object::Handle(to.ptr())); } + void AddExpandoToRehash(const Object& to) { + expandos_to_rehash_.Add(&Object::Handle(to.ptr())); + } void FinalizeTransferables() { for (intptr_t i = 0; i < transferables_from_to_.length(); i += 2) { @@ -464,6 +474,8 @@ class SlowForwardMap : public ForwardMapBase { GrowableArray transferables_from_to_; GrowableArray external_typed_data_; GrowableArray objects_to_rehash_; + GrowableArray expandos_to_rehash_; + GrowableArray weak_properties_; intptr_t fill_cursor_ = 0; DISALLOW_COPY_AND_ASSIGN(SlowForwardMap); @@ -478,7 +490,9 @@ class ObjectCopyBase { heap_(thread->isolate_group()->heap()), class_table_(thread->isolate_group()->class_table()), new_space_(heap_->new_space()), - tmp_(Object::Handle(thread->zone())) {} + tmp_(Object::Handle(thread->zone())), + expando_cid_(Class::GetClassId( + thread->isolate_group()->object_store()->expando_class())) {} ~ObjectCopyBase() {} protected: @@ -590,6 +604,7 @@ class ObjectCopyBase { ClassTable* class_table_; Scavenger* new_space_; Object& tmp_; + intptr_t expando_cid_; const char* exception_msg_ = nullptr; }; @@ -670,7 +685,7 @@ class FastObjectCopyBase : public ObjectCopyBase { return; } - auto to = Forward(tags, value.Decompress(heap_base_)); + auto to = Forward(tags, value_decompressed); StoreCompressedPointerNoBarrier(dst, offset, to); } @@ -708,9 +723,15 @@ class FastObjectCopyBase : public ObjectCopyBase { TransferableTypedDataPtr to) { fast_forward_map_.AddTransferable(from, to); } + void EnqueueWeakProperty(WeakPropertyPtr from) { + fast_forward_map_.AddWeakProperty(from); + } void EnqueueObjectToRehash(ObjectPtr to) { fast_forward_map_.AddObjectToRehash(to); } + void EnqueueExpandoToRehash(ObjectPtr to) { + fast_forward_map_.AddExpandoToRehash(to); + } static void StoreCompressedArrayPointers(intptr_t array_length, ObjectPtr src, @@ -888,9 +909,15 @@ class SlowObjectCopyBase : public ObjectCopyBase { const TransferableTypedData& to) { slow_forward_map_.AddTransferable(from, to); } + void EnqueueWeakProperty(const WeakProperty& from) { + slow_forward_map_.AddWeakProperty(from); + } void EnqueueObjectToRehash(const Object& to) { slow_forward_map_.AddObjectToRehash(to); } + void EnqueueExpandoToRehash(const Object& to) { + slow_forward_map_.AddExpandoToRehash(to); + } void StoreCompressedArrayPointers(intptr_t array_length, const Object& src, @@ -1136,6 +1163,7 @@ class ObjectCopy : public Base { CopyLinkedHashBase<2, typename Types::LinkedHashMap>( from, to, UntagLinkedHashMap(from), UntagLinkedHashMap(to)); } + void CopyLinkedHashSet(typename Types::LinkedHashSet from, typename Types::LinkedHashSet to) { CopyLinkedHashBase<1, typename Types::LinkedHashSet>( @@ -1205,7 +1233,17 @@ class ObjectCopy : public Base { raw_to->offset_in_bytes_ = raw_from->offset_in_bytes_; raw_to->data_ = nullptr; - if (raw_to->typed_data_.Decompress(Base::heap_base_) == Object::null()) { + auto forwarded_backing_store = + raw_to->typed_data_.Decompress(Base::heap_base_); + if (forwarded_backing_store == Marker() || + forwarded_backing_store == Object::null()) { + // Ensure the backing store is never "sentinel" - the scavenger doesn't + // like it. + Base::StoreCompressedPointerNoBarrier( + Types::GetTypedDataViewPtr(to), + OFFSET_OF(UntaggedTypedDataView, typed_data_), Object::null()); + raw_to->length_ = 0; + raw_to->offset_in_bytes_ = 0; ASSERT(Base::exception_msg_ != nullptr); return; } @@ -1257,6 +1295,20 @@ class ObjectCopy : public Base { Base::EnqueueTransferable(from, to); } + void CopyWeakProperty(typename Types::WeakProperty from, + typename Types::WeakProperty to) { + // We store `null`s as keys/values and let the main algorithm know that + // we should check reachability of the key again after the fixpoint (if it + // became reachable, forward the key/value). + Base::StoreCompressedPointerNoBarrier(Types::GetWeakPropertyPtr(to), + OFFSET_OF(UntaggedWeakProperty, key_), + Object::null()); + Base::StoreCompressedPointerNoBarrier( + Types::GetWeakPropertyPtr(to), OFFSET_OF(UntaggedWeakProperty, value_), + Object::null()); + Base::EnqueueWeakProperty(from); + } + #define DEFINE_UNSUPPORTED(clazz) \ void Copy##clazz(typename Types::clazz from, typename Types::clazz to) { \ FATAL("Objects of type " #clazz " should not occur in object graphs"); \ @@ -1291,33 +1343,85 @@ class FastObjectCopy : public ObjectCopy { if (root_copy == Marker()) { return root_copy; } - while (fast_forward_map_.fill_cursor_ < - fast_forward_map_.raw_from_to_.length()) { - const intptr_t index = fast_forward_map_.fill_cursor_; - ObjectPtr from = fast_forward_map_.raw_from_to_[index]; - ObjectPtr to = fast_forward_map_.raw_from_to_[index + 1]; - FastCopyObject(from, to); - if (exception_msg_ != nullptr) { - return root_copy; + auto& from_weak_property = WeakProperty::Handle(zone_); + auto& to_weak_property = WeakProperty::Handle(zone_); + auto& weak_property_key = Object::Handle(zone_); + while (true) { + if (fast_forward_map_.fill_cursor_ == + fast_forward_map_.raw_from_to_.length()) { + break; + } + + // Run fixpoint to copy all objects. + while (fast_forward_map_.fill_cursor_ < + fast_forward_map_.raw_from_to_.length()) { + const intptr_t index = fast_forward_map_.fill_cursor_; + ObjectPtr from = fast_forward_map_.raw_from_to_[index]; + ObjectPtr to = fast_forward_map_.raw_from_to_[index + 1]; + FastCopyObject(from, to); + if (exception_msg_ != nullptr) { + return root_copy; + } + fast_forward_map_.fill_cursor_ += 2; + } + + // Possibly forward values of [WeakProperty]s if keys became reachable. + intptr_t i = 0; + auto& weak_properties = fast_forward_map_.raw_weak_properties_; + while (i < weak_properties.length()) { + from_weak_property = weak_properties[i]; + weak_property_key = + fast_forward_map_.ForwardedObject(from_weak_property.key()); + if (weak_property_key.ptr() != Marker()) { + to_weak_property ^= + fast_forward_map_.ForwardedObject(from_weak_property.ptr()); + + // The key became reachable so we'll change the forwarded + // [WeakProperty]'s key to the new key (it is `null` at this point). + to_weak_property.set_key(weak_property_key); + + // Since the key has become strongly reachable in the copied graph, + // we'll also need to forward the value. + ForwardCompressedPointer(from_weak_property.ptr(), + to_weak_property.ptr(), + OFFSET_OF(UntaggedWeakProperty, value_)); + + // We don't need to process this [WeakProperty] again. + const intptr_t last = weak_properties.length() - 1; + if (i < last) { + weak_properties[i] = weak_properties[last]; + weak_properties.SetLength(last); + continue; + } + } + i++; } - fast_forward_map_.fill_cursor_ += 2; } if (root_copy != Marker()) { - TryBuildArrayOfObjectsToRehash(); + ObjectPtr array; + array = TryBuildArrayOfObjectsToRehash( + fast_forward_map_.raw_objects_to_rehash_); + if (array == Marker()) return root_copy; + raw_objects_to_rehash_ = Array::RawCast(array); + + array = TryBuildArrayOfObjectsToRehash( + fast_forward_map_.raw_expandos_to_rehash_); + if (array == Marker()) return root_copy; + raw_expandos_to_rehash_ = Array::RawCast(array); } return root_copy; } - void TryBuildArrayOfObjectsToRehash() { - const auto& objects_to_rehash = fast_forward_map_.raw_objects_to_rehash_; + ObjectPtr TryBuildArrayOfObjectsToRehash( + const GrowableArray& objects_to_rehash) { const intptr_t length = objects_to_rehash.length(); - if (length == 0) return; + if (length == 0) return Object::null(); const intptr_t size = Array::InstanceSize(length); const uword array_addr = new_space_->TryAllocate(thread_, size); if (array_addr == 0) { exception_msg_ = kFastAllocationFailed; - return; + return Marker(); } const uword header_size = @@ -1333,7 +1437,7 @@ class FastObjectCopy : public ObjectCopy { for (intptr_t i = 0; i < length; ++i) { array_data[i] = objects_to_rehash[i]; } - raw_objects_to_rehash_ = array; + return array; } private: @@ -1365,15 +1469,21 @@ class FastObjectCopy : public ObjectCopy { #else CopyUserdefinedInstance(Instance::RawCast(from), Instance::RawCast(to)); #endif + if (cid == expando_cid_) { + EnqueueExpandoToRehash(to); + } } ArrayPtr raw_objects_to_rehash_ = Array::null(); + ArrayPtr raw_expandos_to_rehash_ = Array::null(); }; class SlowObjectCopy : public ObjectCopy { public: explicit SlowObjectCopy(Thread* thread) - : ObjectCopy(thread), objects_to_rehash_(Array::Handle(thread->zone())) {} + : ObjectCopy(thread), + objects_to_rehash_(Array::Handle(thread->zone())), + expandos_to_rehash_(Array::Handle(thread->zone())) {} ~SlowObjectCopy() {} ObjectPtr ContinueCopyGraphSlow(const Object& root, @@ -1383,32 +1493,76 @@ class SlowObjectCopy : public ObjectCopy { root_copy = Forward(TagsFromUntaggedObject(root.ptr().untag()), root); } + WeakProperty& weak_property = WeakProperty::Handle(Z); Object& from = Object::Handle(Z); Object& to = Object::Handle(Z); - while (slow_forward_map_.fill_cursor_ < - slow_forward_map_.from_to_.length()) { - const intptr_t index = slow_forward_map_.fill_cursor_; - from = slow_forward_map_.from_to_[index]->ptr(); - to = slow_forward_map_.from_to_[index + 1]->ptr(); - CopyObject(from, to); - slow_forward_map_.fill_cursor_ += 2; - if (exception_msg_ != nullptr) { - return Marker(); + while (true) { + if (slow_forward_map_.fill_cursor_ == + slow_forward_map_.from_to_.length()) { + break; + } + + // Run fixpoint to copy all objects. + while (slow_forward_map_.fill_cursor_ < + slow_forward_map_.from_to_.length()) { + const intptr_t index = slow_forward_map_.fill_cursor_; + from = slow_forward_map_.from_to_[index]->ptr(); + to = slow_forward_map_.from_to_[index + 1]->ptr(); + CopyObject(from, to); + slow_forward_map_.fill_cursor_ += 2; + if (exception_msg_ != nullptr) { + return Marker(); + } + } + + // Possibly forward values of [WeakProperty]s if keys became reachable. + intptr_t i = 0; + auto& weak_properties = slow_forward_map_.weak_properties_; + while (i < weak_properties.length()) { + const auto& from_weak_property = *weak_properties[i]; + to = slow_forward_map_.ForwardedObject(from_weak_property.key()); + if (to.ptr() != Marker()) { + weak_property ^= + slow_forward_map_.ForwardedObject(from_weak_property.ptr()); + + // The key became reachable so we'll change the forwarded + // [WeakProperty]'s key to the new key (it is `null` at this point). + weak_property.set_key(to); + + // Since the key has become strongly reachable in the copied graph, + // we'll also need to forward the value. + ForwardCompressedPointer(from_weak_property, weak_property, + OFFSET_OF(UntaggedWeakProperty, value_)); + + // We don't need to process this [WeakProperty] again. + const intptr_t last = weak_properties.length() - 1; + if (i < last) { + weak_properties[i] = weak_properties[last]; + weak_properties.SetLength(last); + continue; + } + } + i++; } } - BuildArrayOfObjectsToRehash(); + + objects_to_rehash_ = + BuildArrayOfObjectsToRehash(slow_forward_map_.objects_to_rehash_); + expandos_to_rehash_ = + BuildArrayOfObjectsToRehash(slow_forward_map_.expandos_to_rehash_); return root_copy.ptr(); } - void BuildArrayOfObjectsToRehash() { - const auto& objects_to_rehash = slow_forward_map_.objects_to_rehash_; + ArrayPtr BuildArrayOfObjectsToRehash( + const GrowableArray& objects_to_rehash) { const intptr_t length = objects_to_rehash.length(); - if (length == 0) return; + if (length == 0) return Array::null(); - objects_to_rehash_ = Array::New(length); + const auto& array = Array::Handle(zone_, Array::New(length)); for (intptr_t i = 0; i < length; ++i) { - objects_to_rehash_.SetAt(i, *objects_to_rehash[i]); + array.SetAt(i, *objects_to_rehash[i]); } + return array.ptr(); } private: @@ -1429,9 +1583,13 @@ class SlowObjectCopy : public ObjectCopy { #else CopyUserdefinedInstance(from, to); #endif + if (cid == expando_cid_) { + EnqueueExpandoToRehash(to); + } } Array& objects_to_rehash_; + Array& expandos_to_rehash_; }; class ObjectGraphCopier { @@ -1492,7 +1650,7 @@ class ObjectGraphCopier { private: ObjectPtr CopyObjectGraphInternal(const Object& root, const char* volatile* exception_msg) { - const auto& result_array = Array::Handle(zone_, Array::New(2)); + const auto& result_array = Array::Handle(zone_, Array::New(3)); if (!root.ptr()->IsHeapObject()) { result_array.SetAt(0, root); return result_array.ptr(); @@ -1523,6 +1681,8 @@ class ObjectGraphCopier { result_array.SetAt(0, result); fast_object_copy_.tmp_ = fast_object_copy_.raw_objects_to_rehash_; result_array.SetAt(1, fast_object_copy_.tmp_); + fast_object_copy_.tmp_ = fast_object_copy_.raw_expandos_to_rehash_; + result_array.SetAt(2, fast_object_copy_.tmp_); HandlifyExternalTypedData(); HandlifyTransferables(); return result_array.ptr(); @@ -1563,6 +1723,7 @@ class ObjectGraphCopier { result_array.SetAt(0, result); result_array.SetAt(1, slow_object_copy_.objects_to_rehash_); + result_array.SetAt(2, slow_object_copy_.expandos_to_rehash_); return result_array.ptr(); } @@ -1572,8 +1733,10 @@ class ObjectGraphCopier { MakeUninitializedNewSpaceObjectsGCSafe(); HandlifyTransferables(); + HandlifyWeakProperties(); HandlifyExternalTypedData(); HandlifyObjectsToReHash(); + HandlifyExpandosToReHash(); HandlifyFromToObjects(); slow_forward_map.fill_cursor_ = fast_forward_map.fill_cursor_; } @@ -1598,46 +1761,35 @@ class ObjectGraphCopier { } } void HandlifyTransferables() { - auto& raw_transferables = - fast_object_copy_.fast_forward_map_.raw_transferables_from_to_; - const auto length = raw_transferables.length(); - if (length > 0) { - auto& transferables = - slow_object_copy_.slow_forward_map_.transferables_from_to_; - transferables.Resize(length); - for (intptr_t i = 0; i < length; i++) { - transferables[i] = - &TransferableTypedData::Handle(Z, raw_transferables[i]); - } - raw_transferables.Clear(); - } + Handlify(&fast_object_copy_.fast_forward_map_.raw_transferables_from_to_, + &slow_object_copy_.slow_forward_map_.transferables_from_to_); + } + void HandlifyWeakProperties() { + Handlify(&fast_object_copy_.fast_forward_map_.raw_weak_properties_, + &slow_object_copy_.slow_forward_map_.weak_properties_); } void HandlifyExternalTypedData() { - auto& raw_external_typed_data = - fast_object_copy_.fast_forward_map_.raw_external_typed_data_to_; - const auto length = raw_external_typed_data.length(); - if (length > 0) { - auto& external_typed_data = - slow_object_copy_.slow_forward_map_.external_typed_data_; - external_typed_data.Resize(length); - for (intptr_t i = 0; i < length; i++) { - external_typed_data[i] = - &ExternalTypedData::Handle(Z, raw_external_typed_data[i]); - } - raw_external_typed_data.Clear(); - } + Handlify(&fast_object_copy_.fast_forward_map_.raw_external_typed_data_to_, + &slow_object_copy_.slow_forward_map_.external_typed_data_); } void HandlifyObjectsToReHash() { - auto& fast_forward_map = fast_object_copy_.fast_forward_map_; - auto& slow_forward_map = slow_object_copy_.slow_forward_map_; - const auto length = fast_forward_map.raw_transferables_from_to_.length(); + Handlify(&fast_object_copy_.fast_forward_map_.raw_objects_to_rehash_, + &slow_object_copy_.slow_forward_map_.objects_to_rehash_); + } + void HandlifyExpandosToReHash() { + Handlify(&fast_object_copy_.fast_forward_map_.raw_expandos_to_rehash_, + &slow_object_copy_.slow_forward_map_.expandos_to_rehash_); + } + template + void Handlify(GrowableArray* from, + GrowableArray* to) { + const auto length = from->length(); if (length > 0) { - slow_forward_map.objects_to_rehash_.Resize(length); + to->Resize(length); for (intptr_t i = 0; i < length; i++) { - slow_forward_map.objects_to_rehash_[i] = - &Object::Handle(Z, fast_forward_map.raw_objects_to_rehash_[i]); + (*to)[i] = &HandleType::Handle(Z, (*from)[i]); } - fast_forward_map.raw_objects_to_rehash_.Clear(); + from->Clear(); } } void HandlifyFromToObjects() { diff --git a/runtime/vm/raw_object.h b/runtime/vm/raw_object.h index e50a979409c..f99afbe06d9 100644 --- a/runtime/vm/raw_object.h +++ b/runtime/vm/raw_object.h @@ -3206,6 +3206,8 @@ class UntaggedWeakProperty : public UntaggedInstance { friend class Scavenger; template friend class ScavengerVisitorBase; + friend class FastObjectCopy; // For OFFSET_OF + friend class SlowObjectCopy; // For OFFSET_OF }; // MirrorReferences are used by mirrors to hold reflectees that are VM diff --git a/runtime/vm/symbols.h b/runtime/vm/symbols.h index 76d0070e293..03dd529992e 100644 --- a/runtime/vm/symbols.h +++ b/runtime/vm/symbols.h @@ -415,7 +415,6 @@ class ObjectPointerVisitor; V(_objectNoSuchMethod, "_objectNoSuchMethod") \ V(_objectToString, "_objectToString") \ V(_onData, "_onData") \ - V(_rehash, "_rehash") \ V(_rehashObjects, "_rehashObjects") \ V(_resultOrListeners, "_resultOrListeners") \ V(_runExtension, "_runExtension") \ diff --git a/sdk/lib/_internal/vm/lib/expando_patch.dart b/sdk/lib/_internal/vm/lib/expando_patch.dart index 36c65df5c73..f70710297db 100644 --- a/sdk/lib/_internal/vm/lib/expando_patch.dart +++ b/sdk/lib/_internal/vm/lib/expando_patch.dart @@ -4,6 +4,16 @@ // part of "core_patch.dart"; +// This function takes care of rehashing of the expandos in [objects]. We +// do this eagerly after snapshot deserialization. +@pragma("vm:entry-point", "call") +void _rehashObjects(List objects) { + final int length = objects.length; + for (int i = 0; i < length; ++i) { + objects[i]._rehash(); + } +} + @patch @pragma("vm:entry-point") class Expando { @@ -96,7 +106,6 @@ class Expando { this[object] = value; // Recursively add the value. } - @pragma("vm:entry-point", "call") _rehash() { // Determine the population count of the map to allocate an appropriately // sized map below. diff --git a/tests/lib/isolate/weak_property_message_1_test.dart b/tests/lib/isolate/weak_property_message_1_test.dart index 2f4d344c2dc..494622284b1 100644 --- a/tests/lib/isolate/weak_property_message_1_test.dart +++ b/tests/lib/isolate/weak_property_message_1_test.dart @@ -4,7 +4,9 @@ // See https://github.com/dart-lang/sdk/issues/25559 -import "dart:async"; +// VMOptions=--no-enable-isolate-groups +// VMOptions=--enable-isolate-groups + import "dart:isolate"; import "package:async_helper/async_helper.dart"; @@ -30,7 +32,6 @@ main() { asyncEnd(); }); - var key1 = new Object(); var key2 = new Object(); var key3 = new Object(); diff --git a/tests/lib/isolate/weak_property_message_2_test.dart b/tests/lib/isolate/weak_property_message_2_test.dart index 236bcfe3976..fd0dbc6a0c5 100644 --- a/tests/lib/isolate/weak_property_message_2_test.dart +++ b/tests/lib/isolate/weak_property_message_2_test.dart @@ -4,12 +4,13 @@ // See https://github.com/dart-lang/sdk/issues/25559 -import "dart:async"; +// VMOptions=--no-enable-isolate-groups +// VMOptions=--enable-isolate-groups + import "dart:developer"; import "dart:isolate"; import "package:async_helper/async_helper.dart"; -import "package:expect/expect.dart"; main() { asyncStart(); @@ -23,12 +24,11 @@ main() { asyncEnd(); }); - var unwrittenKey = new Object(); var expando = new Expando(); expando[unwrittenKey] = new UserTag("cant send this"); port.sendPort.send(expando); - print(unwrittenKey); // Ensure [unwrittenKey] is live during [send]. + print(unwrittenKey); // Ensure [unwrittenKey] is live during [send]. } diff --git a/tests/lib_2/isolate/weak_property_message_1_test.dart b/tests/lib_2/isolate/weak_property_message_1_test.dart index fa8a6d1d926..b58e2610cd3 100644 --- a/tests/lib_2/isolate/weak_property_message_1_test.dart +++ b/tests/lib_2/isolate/weak_property_message_1_test.dart @@ -6,7 +6,9 @@ // @dart = 2.9 -import "dart:async"; +// VMOptions=--no-enable-isolate-groups +// VMOptions=--enable-isolate-groups + import "dart:isolate"; import "package:async_helper/async_helper.dart"; @@ -32,7 +34,6 @@ main() { asyncEnd(); }); - var key1 = new Object(); var key2 = new Object(); var key3 = new Object(); diff --git a/tests/lib_2/isolate/weak_property_message_2_test.dart b/tests/lib_2/isolate/weak_property_message_2_test.dart index 7b5ad95d633..353a8f5e0f1 100644 --- a/tests/lib_2/isolate/weak_property_message_2_test.dart +++ b/tests/lib_2/isolate/weak_property_message_2_test.dart @@ -6,12 +6,13 @@ // @dart = 2.9 -import "dart:async"; +// VMOptions=--no-enable-isolate-groups +// VMOptions=--enable-isolate-groups + import "dart:developer"; import "dart:isolate"; import "package:async_helper/async_helper.dart"; -import "package:expect/expect.dart"; main() { asyncStart(); @@ -25,12 +26,11 @@ main() { asyncEnd(); }); - var unwrittenKey = new Object(); var expando = new Expando(); expando[unwrittenKey] = new UserTag("cant send this"); port.sendPort.send(expando); - print(unwrittenKey); // Ensure [unwrittenKey] is live during [send]. + print(unwrittenKey); // Ensure [unwrittenKey] is live during [send]. }