From fa68e858ba4a5e9f09a51279e4290629e0373c25 Mon Sep 17 00:00:00 2001 From: Milos Kotlar Date: Wed, 29 Mar 2023 23:40:28 +0200 Subject: [PATCH 1/3] Implement GC descriptor for structs with InlineArrayAttribute and add Mono GC test --- src/mono/mono/metadata/object.c | 118 +++++++++--------- .../InlineArray/InlineArrayValid.cs | 47 +++++++ 2 files changed, 107 insertions(+), 58 deletions(-) diff --git a/src/mono/mono/metadata/object.c b/src/mono/mono/metadata/object.c index 90b2cf2420d13a..ada301dd67faac 100644 --- a/src/mono/mono/metadata/object.c +++ b/src/mono/mono/metadata/object.c @@ -874,72 +874,74 @@ compute_class_bitmap (MonoClass *klass, gsize *bitmap, int size, int offset, int /* special/collectible static */ continue; - pos = field_offset / TARGET_SIZEOF_VOID_P; - pos += offset; - - type = mono_type_get_underlying_type (field->type); - - switch (type->type) { - case MONO_TYPE_U: - case MONO_TYPE_I: - case MONO_TYPE_PTR: - case MONO_TYPE_FNPTR: - break; - case MONO_TYPE_STRING: - case MONO_TYPE_SZARRAY: - case MONO_TYPE_CLASS: - case MONO_TYPE_OBJECT: - case MONO_TYPE_ARRAY: - g_assert ((m_field_get_offset (field) % wordsize) == 0); - - g_assert (pos < GINT_TO_UINT32(size) || pos <= GINT_TO_UINT32(max_size)); - bitmap [pos / BITMAP_EL_SIZE] |= ((gsize)1) << (pos % BITMAP_EL_SIZE); - *max_set = MAX (GINT_TO_UINT32(*max_set), pos); - break; - case MONO_TYPE_GENERICINST: - if (!mono_type_generic_inst_is_valuetype (type)) { + guint32 field_iter = 1; + guint32 field_instance_offset = field_offset; + // If struct has InlineArray attribute, iterate `length` times to set a bitmap + if (m_class_is_inlinearray (p)) + field_iter = m_class_inlinearray_value (p); + + while (field_iter) { + pos = field_instance_offset / TARGET_SIZEOF_VOID_P; + pos += offset; + + type = mono_type_get_underlying_type (field->type); + + switch (type->type) { + case MONO_TYPE_U: + case MONO_TYPE_I: + case MONO_TYPE_PTR: + case MONO_TYPE_FNPTR: + break; + case MONO_TYPE_STRING: + case MONO_TYPE_SZARRAY: + case MONO_TYPE_CLASS: + case MONO_TYPE_OBJECT: + case MONO_TYPE_ARRAY: g_assert ((m_field_get_offset (field) % wordsize) == 0); + g_assert (pos < GINT_TO_UINT32(size) || pos <= GINT_TO_UINT32(max_size)); bitmap [pos / BITMAP_EL_SIZE] |= ((gsize)1) << (pos % BITMAP_EL_SIZE); *max_set = MAX (GINT_TO_UINT32(*max_set), pos); break; - } else { - /* fall through */ + case MONO_TYPE_GENERICINST: + if (!mono_type_generic_inst_is_valuetype (type)) { + g_assert ((m_field_get_offset (field) % wordsize) == 0); + + bitmap [pos / BITMAP_EL_SIZE] |= ((gsize)1) << (pos % BITMAP_EL_SIZE); + *max_set = MAX (GINT_TO_UINT32(*max_set), pos); + break; + } else { + /* fall through */ + } + case MONO_TYPE_TYPEDBYREF: + case MONO_TYPE_VALUETYPE: { + MonoClass *fclass = mono_class_from_mono_type_internal (field->type); + if (m_class_has_references (fclass)) { + /* remove the object header */ + compute_class_bitmap (fclass, bitmap, size, pos - MONO_OBJECT_HEADER_BITS, max_set, FALSE); + } + break; } - case MONO_TYPE_TYPEDBYREF: - case MONO_TYPE_VALUETYPE: { - MonoClass *fclass = mono_class_from_mono_type_internal (field->type); - if (m_class_has_references (fclass)) { - /* remove the object header */ - compute_class_bitmap (fclass, bitmap, size, pos - MONO_OBJECT_HEADER_BITS, max_set, FALSE); + case MONO_TYPE_I1: + case MONO_TYPE_U1: + case MONO_TYPE_I2: + case MONO_TYPE_U2: + case MONO_TYPE_I4: + case MONO_TYPE_U4: + case MONO_TYPE_I8: + case MONO_TYPE_U8: + case MONO_TYPE_R4: + case MONO_TYPE_R8: + case MONO_TYPE_BOOLEAN: + case MONO_TYPE_CHAR: + break; + default: + g_error ("compute_class_bitmap: Invalid type %x for field %s:%s\n", type->type, mono_type_get_full_name (m_field_get_parent (field)), field->name); + break; } - break; - } - case MONO_TYPE_I1: - case MONO_TYPE_U1: - case MONO_TYPE_I2: - case MONO_TYPE_U2: - case MONO_TYPE_I4: - case MONO_TYPE_U4: - case MONO_TYPE_I8: - case MONO_TYPE_U8: - case MONO_TYPE_R4: - case MONO_TYPE_R8: - case MONO_TYPE_BOOLEAN: - case MONO_TYPE_CHAR: - break; - default: - g_error ("compute_class_bitmap: Invalid type %x for field %s:%s\n", type->type, mono_type_get_full_name (m_field_get_parent (field)), field->name); - break; - } - - // A struct with the inline array attribute is handled in the same way as an array - if (m_class_is_inlinearray (klass)) { - g_assert ((m_field_get_offset (field) % wordsize) == 0); - g_assert (pos < GINT_TO_UINT32(size) || pos <= GINT_TO_UINT32(max_size)); - bitmap [pos / BITMAP_EL_SIZE] |= ((gsize)1) << (pos % BITMAP_EL_SIZE); - *max_set = MAX (GINT_TO_UINT32(*max_set), pos); + field_instance_offset += field_offset; + field_iter--; } } if (static_fields) diff --git a/src/tests/Loader/classloader/InlineArray/InlineArrayValid.cs b/src/tests/Loader/classloader/InlineArray/InlineArrayValid.cs index 705f32e13b6c9b..f632f210d06fdf 100644 --- a/src/tests/Loader/classloader/InlineArray/InlineArrayValid.cs +++ b/src/tests/Loader/classloader/InlineArray/InlineArrayValid.cs @@ -172,6 +172,21 @@ struct ObjShortArr [UnscopedRef] public ref (object o, short s) this[int i] => ref Unsafe.Add(ref element, i); + + [MethodImpl(MethodImplOptions.NoInlining)] + public static ObjShortArr CreateArray(int recCount) { + if (recCount > 0) { + return CreateArray(recCount-1); + } else { + var arr = new ObjShortArr(); + for (short i = 0; i < ObjShortArr.Length; i++) + { + arr[i].o = i; + arr[i].s = (short)(i + 1); + } + return arr; + } + } } [Fact] @@ -384,4 +399,36 @@ public static void GCDescOpt() Assert.Equal(1, *gcSeriesPtr); } } + + // ====================== MonoGCDesc ========================================================== + + class Holder { + public ObjShortArr arr; + } + + static Holder CreateArray() { + var arr = ObjShortArr.CreateArray(100); + var holder = new Holder(); + holder.arr = arr; + return holder; + } + + [Fact] + [SkipOnCoreClr("Mono-specific implementation details.")] + public static void MonoGCDescOpt() + { + Console.WriteLine($"{nameof(MonoGCDescOpt)}..."); + + var holder = CreateArray(); + + GC.Collect(2, GCCollectionMode.Forced, true, true); + + MakeGarbage (); + + for (short i = 0; i < ObjShortArr.Length; i++) + { + Assert.Equal(i, holder.arr[i].o); + Assert.Equal(i + 1, holder.arr[i].s); + } + } } From 6ffb865bbf7f35ada473d91ccd428a85de9c27e5 Mon Sep 17 00:00:00 2001 From: Milos Kotlar Date: Thu, 30 Mar 2023 10:40:08 +0200 Subject: [PATCH 2/3] Enable GC test on CoreCLR --- src/tests/Loader/classloader/InlineArray/InlineArrayValid.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/tests/Loader/classloader/InlineArray/InlineArrayValid.cs b/src/tests/Loader/classloader/InlineArray/InlineArrayValid.cs index f632f210d06fdf..82ebaa009db636 100644 --- a/src/tests/Loader/classloader/InlineArray/InlineArrayValid.cs +++ b/src/tests/Loader/classloader/InlineArray/InlineArrayValid.cs @@ -414,7 +414,6 @@ static Holder CreateArray() { } [Fact] - [SkipOnCoreClr("Mono-specific implementation details.")] public static void MonoGCDescOpt() { Console.WriteLine($"{nameof(MonoGCDescOpt)}..."); @@ -423,7 +422,7 @@ public static void MonoGCDescOpt() GC.Collect(2, GCCollectionMode.Forced, true, true); - MakeGarbage (); + MakeGarbage(); for (short i = 0; i < ObjShortArr.Length; i++) { From 17a38f388918f99fcb3f693923b0bd0e2dde5537 Mon Sep 17 00:00:00 2001 From: Milos Kotlar Date: Thu, 30 Mar 2023 10:52:16 +0200 Subject: [PATCH 3/3] Add warning when field_iter too large --- src/mono/mono/metadata/object.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/mono/mono/metadata/object.c b/src/mono/mono/metadata/object.c index ada301dd67faac..c157ffc37332e3 100644 --- a/src/mono/mono/metadata/object.c +++ b/src/mono/mono/metadata/object.c @@ -880,6 +880,9 @@ compute_class_bitmap (MonoClass *klass, gsize *bitmap, int size, int offset, int if (m_class_is_inlinearray (p)) field_iter = m_class_inlinearray_value (p); + if (field_iter > 500) + g_warning ("Large number of iterations detected when creating a GC bitmap, might affect performance."); + while (field_iter) { pos = field_instance_offset / TARGET_SIZEOF_VOID_P; pos += offset;