Merge pull request #3647 from BrzVlad/fix-sgen-internal-alloc

[sgen] Fix sgen internal alloc

diff --git a/mono/sgen/sgen-internal.c b/mono/sgen/sgen-internal.c
index ca30670b71ab4e712d39639fc1c7e6bd2e2ba307..84ea7d4549a9b72c2179c676fc5d1dccf9a63feb 100644 (file)
--- a/mono/sgen/sgen-internal.c
+++ b/mono/sgen/sgen-internal.c
@@ -17,19 +17,35 @@
 #include "mono/sgen/sgen-memory-governor.h"
 #include "mono/sgen/sgen-client.h"
 
-/* keep each size a multiple of ALLOC_ALIGN */
+/*
+ * When allocating sgen memory we choose the allocator with the smallest slot size
+ * that can fit our requested size. These slots are allocated within a block that
+ * can contain at least 2 slots of the specific size.
+ *
+ * Currently, slots from 8 to 2044/2040 are allocated inside 4096 sized blocks,
+ * 2728 to 4092/4088 inside 8192 sized blocks, and higher inside 16384 sized
+ * blocks. We also need to make sure the slots are pointer size aligned so we
+ * don't allocate unaligned memory.
+ *
+ * The computation of these sizes spawns from two basic rules :
+ *     - if we use slots of size s1 that fit n times in a block, it is illogical
+ * to use another slot of size s2 which also fits the same n times in a block.
+ *     - if we use slots of size s1 that fit n times in a block, there is no
+ * s2 > s1 that can fit n times in the block. That would mean we are wasting memory
+ * when allocating size S where s1 < S <= s2.
+ */
 #if SIZEOF_VOID_P == 4
 static const int allocator_sizes [] = {
           8,   16,   24,   32,   40,   48,   64,   80,
-         96,  128,  160,  192,  224,  248,  296,  320,
-        384,  448,  504,  528,  584,  680,  816, 1088,
-       1360, 2046, 2336, 2728, 3272, 4094, 5456, 8190 };
+         96,  124,  160,  192,  224,  252,  292,  340,
+        408,  452,  508,  584,  680,  816, 1020,
+       1364, 2044, 2728, 4092, 5460, 8188 };
 #else
 static const int allocator_sizes [] = {
           8,   16,   24,   32,   40,   48,   64,   80,
-         96,  128,  160,  192,  224,  248,  296,  320,
-        384,  448,  504,  528,  584,  680,  816, 1088,
-       1360, 2044, 2336, 2728, 3272, 4092, 5456, 8188 };
+         96,  128,  160,  192,  224,  248,  288,  336,
+        368,  448,  504,  584,  680,  816, 1016,
+       1360, 2040, 2728, 4088, 5456, 8184 };
 #endif
 
 #define NUM_ALLOCATORS (sizeof (allocator_sizes) / sizeof (int))
@@ -173,6 +189,8 @@ sgen_alloc_internal_dynamic (size_t size, int type, gboolean assert_on_failure)
                        sgen_assert_memory_alloc (NULL, size, description_for_type (type));
                memset (p, 0, size);
        }
+
+       SGEN_ASSERT (0, !(((mword)p) & (sizeof(gpointer) - 1)), "Why do we allocate unaligned addresses ?");
        return p;
 }
 
@@ -206,6 +224,8 @@ sgen_alloc_internal (int type)
        p = mono_lock_free_alloc (&allocators [index]);
        memset (p, 0, size);
 
+       SGEN_ASSERT (0, !(((mword)p) & (sizeof(gpointer) - 1)), "Why do we allocate unaligned addresses ?");
+
        return p;
 }
 
@@ -264,10 +284,11 @@ sgen_init_internal_allocator (void)
        }
 
        for (size = mono_pagesize (); size <= LOCK_FREE_ALLOC_SB_MAX_SIZE; size <<= 1) {
-               int max_size = LOCK_FREE_ALLOC_SB_USABLE_SIZE (size) / 2;
+               int max_size = (LOCK_FREE_ALLOC_SB_USABLE_SIZE (size) / 2) & ~(SIZEOF_VOID_P - 1);
                /*
                 * we assert that allocator_sizes contains the biggest possible object size
-                * per block (4K => 4080 / 2 = 2040, 8k => 8176 / 2 = 4088, 16k => 16368 / 2 = 8184 on 64bits),
+                * per block which has to be an aligned address.
+                * (4K => 2040, 8k => 4088, 16k => 8184 on 64bits),
                 * so that we do not get different block sizes for sizes that should go to the same one
                 */
                g_assert (allocator_sizes [index_for_size (max_size)] == max_size);

diff --git a/mono/sgen/sgen-marksweep-drain-gray-stack.h b/mono/sgen/sgen-marksweep-drain-gray-stack.h
index 3c96a2f0106bfc78cf20bca223315e7bd3f8c63a..d6460111a0f4094c25b90ca38e8f2445e0d6edd8 100644 (file)
--- a/mono/sgen/sgen-marksweep-drain-gray-stack.h
+++ b/mono/sgen/sgen-marksweep-drain-gray-stack.h
@@ -142,7 +142,8 @@ COPY_OR_MARK_FUNCTION_NAME (GCObject **ptr, GCObject *obj, SgenGrayQueue *queue)
 
                SGEN_ASSERT (9, !SGEN_VTABLE_IS_PINNED (vtable_word), "Pinned object in non-pinned block?");
 
-               desc = sgen_vtable_get_descriptor ((GCVTable)vtable_word);
+               /* We untag the vtable for concurrent M&S, in case bridge is running and it tagged it */
+               desc = sgen_vtable_get_descriptor ((GCVTable)SGEN_POINTER_UNTAG_VTABLE (vtable_word));
                type = desc & DESC_TYPE_MASK;
 
                if (sgen_safe_object_is_small (obj, type)) {
@@ -183,7 +184,7 @@ COPY_OR_MARK_FUNCTION_NAME (GCObject **ptr, GCObject *obj, SgenGrayQueue *queue)
 
                        sgen_los_pin_object (obj);
                        if (SGEN_OBJECT_HAS_REFERENCES (obj))
-                               GRAY_OBJECT_ENQUEUE (queue, obj, sgen_obj_get_descriptor (obj));
+                               GRAY_OBJECT_ENQUEUE (queue, obj, desc);
                }
                return FALSE;
        }