From 565bc4830249a35e628fa0fdc6f80edea83b398a Mon Sep 17 00:00:00 2001 From: Ryan Macnak Date: Thu, 16 May 2024 19:30:58 +0000 Subject: [PATCH] [vm, gc] Use FreeListElement as the filler object for array truncation. This is consistent with new-space sweeping using FreeListElement as the filler object, and allows for stronger asserts in incremental compaction. TEST=ci Change-Id: I43a87a46fb1211a88589cf4a349ada7b556e4c11 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/366885 Reviewed-by: Alexander Aprelev Commit-Queue: Ryan Macnak --- runtime/vm/object.cc | 67 +++++++-------------------------------- runtime/vm/object_test.cc | 49 +++++++++------------------- 2 files changed, 27 insertions(+), 89 deletions(-) diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc index d2258eda029..f9d2e00a1e9 100644 --- a/runtime/vm/object.cc +++ b/runtime/vm/object.cc @@ -1605,8 +1605,8 @@ void Object::set_vm_isolate_snapshot_object_table(const Array& table) { // Make unused space in an object whose type has been transformed safe // for traversing during GC. -// The unused part of the transformed object is marked as an TypedDataInt8Array -// object. +// The unused part of the transformed object is marked as a FreeListElement +// object that is not inserted into to the freelist. void Object::MakeUnusedSpaceTraversable(const Object& obj, intptr_t original_size, intptr_t used_size) { @@ -1615,62 +1615,19 @@ void Object::MakeUnusedSpaceTraversable(const Object& obj, ASSERT(original_size >= used_size); if (original_size > used_size) { intptr_t leftover_size = original_size - used_size; - uword addr = UntaggedObject::ToAddr(obj.ptr()) + used_size; - if (leftover_size >= TypedData::InstanceSize(0)) { - // Update the leftover space as a TypedDataInt8Array object. - TypedDataPtr raw = - static_cast(UntaggedObject::FromAddr(addr)); - uword new_tags = - UntaggedObject::ClassIdTag::update(kTypedDataInt8ArrayCid, 0); - new_tags = UntaggedObject::SizeTag::update(leftover_size, new_tags); - const bool is_old = obj.ptr()->IsOldObject(); - new_tags = UntaggedObject::AlwaysSetBit::update(true, new_tags); - new_tags = UntaggedObject::NotMarkedBit::update(true, new_tags); - new_tags = - UntaggedObject::OldAndNotRememberedBit::update(is_old, new_tags); - new_tags = UntaggedObject::NewBit::update(!is_old, new_tags); - // On architectures with a relaxed memory model, the concurrent marker may - // observe the write of the filler object's header before observing the - // new array length, and so treat it as a pointer. Ensure it is a Smi so - // the marker won't dereference it. - ASSERT((new_tags & kSmiTagMask) == kSmiTag); - - intptr_t leftover_len = (leftover_size - TypedData::InstanceSize(0)); - ASSERT(TypedData::InstanceSize(leftover_len) == leftover_size); - raw->untag()->set_length( - Smi::New(leftover_len)); - raw->untag()->tags_ = new_tags; - raw->untag()->RecomputeDataField(); + if (obj.ptr()->IsNewObject()) { + FreeListElement::AsElementNew(addr, leftover_size); } else { - // Update the leftover space as a basic object. - ASSERT(leftover_size == Object::InstanceSize()); - ObjectPtr raw = static_cast(UntaggedObject::FromAddr(addr)); - uword new_tags = UntaggedObject::ClassIdTag::update(kInstanceCid, 0); - new_tags = UntaggedObject::SizeTag::update(leftover_size, new_tags); - const bool is_old = obj.ptr()->IsOldObject(); - new_tags = UntaggedObject::AlwaysSetBit::update(true, new_tags); - new_tags = UntaggedObject::NotMarkedBit::update(true, new_tags); - new_tags = - UntaggedObject::OldAndNotRememberedBit::update(is_old, new_tags); - new_tags = UntaggedObject::NewBit::update(!is_old, new_tags); - // On architectures with a relaxed memory model, the concurrent marker may - // observe the write of the filler object's header before observing the - // new array length, and so treat it as a pointer. Ensure it is a Smi so - // the marker won't dereference it. - ASSERT((new_tags & kSmiTagMask) == kSmiTag); - - // The array might have an uninitialized alignment gap since the visitors - // for Arrays are precise based on element count, but the visitors for - // Instance are based on the size rounded to the allocation unit, so we - // need to ensure the alignment gap is initialized. - for (intptr_t offset = Instance::UnroundedSize(); - offset < Instance::InstanceSize(); offset += sizeof(uword)) { - reinterpret_cast*>(addr + offset) - ->store(0, std::memory_order_release); - } - raw->untag()->tags_ = new_tags; + FreeListElement::AsElement(addr, leftover_size); } + // On architectures with a relaxed memory model, the concurrent marker may + // observe the write of the filler object's header before observing the + // new array length, and so treat it as a pointer. Ensure it is a Smi so + // the marker won't dereference it. + ASSERT((*reinterpret_cast(addr) & kSmiTagMask) == kSmiTag); + ASSERT((*reinterpret_cast(addr + kWordSize) & kSmiTagMask) == + kSmiTag); } } diff --git a/runtime/vm/object_test.cc b/runtime/vm/object_test.cc index 8f92df0145c..41c7f280741 100644 --- a/runtime/vm/object_test.cc +++ b/runtime/vm/object_test.cc @@ -2095,9 +2095,8 @@ ISOLATE_UNIT_TEST_CASE(GrowableObjectArray) { // Test the MakeFixedLength functionality to make sure the resulting array // object is properly setup. - // 1. Should produce an array of length 2 and a left over int8 array. + // 1. Should produce an array of length 2 and a filler of minimal size. Array& new_array = Array::Handle(); - TypedData& left_over_array = TypedData::Handle(); Object& obj = Object::Handle(); uword addr = 0; intptr_t used_size = 0; @@ -2117,19 +2116,12 @@ ISOLATE_UNIT_TEST_CASE(GrowableObjectArray) { new_array ^= obj.ptr(); EXPECT_EQ(2, new_array.Length()); addr += used_size; - obj = UntaggedObject::FromAddr(addr); -#if defined(DART_COMPRESSED_POINTERS) - // In compressed pointer mode, the TypedData doesn't fit. - EXPECT(obj.IsInstance()); -#else - EXPECT(obj.IsTypedData()); - left_over_array ^= obj.ptr(); - EXPECT_EQ(4 * kWordSize - TypedData::InstanceSize(0), - left_over_array.Length()); -#endif + ObjectPtr filler = UntaggedObject::FromAddr(addr); + EXPECT(filler->IsFreeListElement()); + EXPECT_EQ(filler->untag()->HeapSize(), + Array::InstanceSize(kArrayLen + 1) - used_size); - // 2. Should produce an array of length 3 and a left over int8 array or - // instance. + // 2. Should produce an array of length 3 and a filler object. array = GrowableObjectArray::New(kArrayLen); EXPECT_EQ(kArrayLen, array.Capacity()); EXPECT_EQ(0, array.Length()); @@ -2145,17 +2137,12 @@ ISOLATE_UNIT_TEST_CASE(GrowableObjectArray) { new_array ^= obj.ptr(); EXPECT_EQ(3, new_array.Length()); addr += used_size; - obj = UntaggedObject::FromAddr(addr); - if (TypedData::InstanceSize(0) <= 2 * kCompressedWordSize) { - EXPECT(obj.IsTypedData()); - left_over_array ^= obj.ptr(); - EXPECT_EQ(2 * kCompressedWordSize - TypedData::InstanceSize(0), - left_over_array.Length()); - } else { - EXPECT(obj.IsInstance()); - } + filler = UntaggedObject::FromAddr(addr); + EXPECT(filler->IsFreeListElement()); + EXPECT_EQ(filler->untag()->HeapSize(), + Array::InstanceSize(kArrayLen) - used_size); - // 3. Should produce an array of length 1 and a left over int8 array. + // 3. Should produce an array of length 1 and a filler object. array = GrowableObjectArray::New(kArrayLen + 3); EXPECT_EQ((kArrayLen + 3), array.Capacity()); EXPECT_EQ(0, array.Length()); @@ -2171,16 +2158,10 @@ ISOLATE_UNIT_TEST_CASE(GrowableObjectArray) { new_array ^= obj.ptr(); EXPECT_EQ(1, new_array.Length()); addr += used_size; - obj = UntaggedObject::FromAddr(addr); -#if defined(DART_COMPRESSED_POINTERS) - // In compressed pointer mode, the TypedData doesn't fit. - EXPECT(obj.IsInstance()); -#else - EXPECT(obj.IsTypedData()); - left_over_array ^= obj.ptr(); - EXPECT_EQ(8 * kWordSize - TypedData::InstanceSize(0), - left_over_array.Length()); -#endif + filler = UntaggedObject::FromAddr(addr); + EXPECT(filler->IsFreeListElement()); + EXPECT_EQ(filler->untag()->HeapSize(), + Array::InstanceSize(kArrayLen + 3) - used_size); // 4. Verify that GC can handle the filler object for a large array. array = GrowableObjectArray::New((1 * MB) >> kWordSizeLog2);