gc: new maybe_gc_mark gc vtable

fixes --gc ms which called the ms2! maybe_mark_and_sweep function from
fixed_allocator: allocate_new_pool_arena.

Closes GH #1143

src/gc/fixed_allocator.c

@@ -538,7 +538,7 @@ allocate_new_pool_arena(PARROT_INTERP, ARGMOD(Pool_Allocator *pool))
     const size_t total_size = arena_size(pool);
 
     /* Run a GC if needed */
-    Parrot_gc_maybe_mark_and_sweep(interp, GC_trace_stack_FLAG);
+    interp->gc_sys->maybe_gc_mark(interp, GC_trace_stack_FLAG);
 
     new_arena = (Pool_Allocator_Arena *)mem_sys_allocate_zeroed(total_size);
 

src/gc/gc_gms.c

@@ -106,10 +106,10 @@ TBD
 #include "parrot/parrot.h"
 #include "parrot/gc_api.h"
 #include "parrot/pointer_array.h"
-#include "parrot/list.h"        /* only for MEMORY_DEBUG */
 #include "gc_private.h"
 #include "fixed_allocator.h"
 #ifdef MEMORY_DEBUG
+#  include "parrot/list.h"
 #  include "parrot/runcore_trace.h"
 #endif
 
@@ -371,6 +371,9 @@ static void gc_gms_iterate_live_strings(PARROT_INTERP,
     ARGIN_NULLOK(void *data))
         __attribute__nonnull__(1);
 
+static void gc_gms_maybe_mark_and_sweep(PARROT_INTERP, UINTVAL flags)
+        __attribute__nonnull__(1);
+
 static void gc_gms_mark_and_sweep(PARROT_INTERP, UINTVAL flags)
         __attribute__nonnull__(1);
 
@@ -681,6 +684,7 @@ Parrot_gc_gms_init(PARROT_INTERP, ARGIN(Parrot_GC_Init_Args *args))
     /* We have to transfer ownership of memory to parent interp in threaded parrot */
     interp->gc_sys->finalize_gc_system = NULL; /* gc_gms_finalize; */
 
+    interp->gc_sys->maybe_gc_mark               = gc_gms_maybe_mark_and_sweep;
     interp->gc_sys->do_gc_mark                  = gc_gms_mark_and_sweep;
     interp->gc_sys->compact_string_pool         = gc_gms_compact_memory_pool;
 
@@ -1483,23 +1487,23 @@ gc_gms_finalize(PARROT_INTERP)
 
 /*
 
-=item C<gc_gms_maybe_mark_and_sweep(PARROT_INTERP)>
+=item C<gc_gms_maybe_mark_and_sweep(PARROT_INTERP, UINTVAL flags)>
 
 Maybe M&S. Depends on total allocated memory, memory allocated since last alloc.
 
 =cut
 
 */
 
-#define gc_gms_maybe_mark_and_sweep(i) \
-    do { \
-        MarkSweep_GC * const _self = (MarkSweep_GC *)(i)->gc_sys->gc_private; \
-    \
-        /* Collect every gc_threshold. */ \
-        if (!_self->gc_mark_block_level \
-        &&  (i)->gc_sys->stats.mem_used_last_collect > _self->gc_threshold) \
-            gc_gms_mark_and_sweep(interp, 0); \
-    } while (0)
+static void
+gc_gms_maybe_mark_and_sweep(PARROT_INTERP, UINTVAL flags) {
+    MarkSweep_GC * const self = (MarkSweep_GC *)(interp)->gc_sys->gc_private;
+
+    /* Collect every gc_threshold. */
+    if (!self->gc_mark_block_level
+    && interp->gc_sys->stats.mem_used_last_collect > self->gc_threshold)
+        gc_gms_mark_and_sweep(interp, flags);
+}
 
 PARROT_MALLOC
 PARROT_CAN_RETURN_NULL
@@ -1511,7 +1515,7 @@ gc_gms_allocate_pmc_header(PARROT_INTERP, SHIM(UINTVAL flags))
     Pool_Allocator   * const pool = self->pmc_allocator;
     pmc_alloc_struct * item = NULL;
 
-    gc_gms_maybe_mark_and_sweep(interp);
+    gc_gms_maybe_mark_and_sweep(interp, 0);
 
     if (interp->thread_data)
         LOCK(interp->thread_data->interp_lock);
@@ -1647,7 +1651,7 @@ gc_gms_allocate_string_header(PARROT_INTERP, SHIM(UINTVAL flags))
     string_alloc_struct *item;
     STRING              *ret;
 
-    gc_gms_maybe_mark_and_sweep(interp);
+    gc_gms_maybe_mark_and_sweep(interp, 0);
 
     if (interp->thread_data)
         LOCK(interp->thread_data->interp_lock);
@@ -2380,9 +2384,11 @@ gc_gms_print_stats_always(PARROT_INTERP, ARGIN(const char* header))
     fprintf(stderr, ", attrs: %6lu",
             (unsigned long)Parrot_gc_fixed_allocator_allocated_memory(interp,
               self->fixed_size_allocator));
-    fprintf(stderr, ", parent: 0x%lx, tid: %3d",
-            (unsigned long)interp->parent_interpreter,
-            interp->thread_data ? (signed)interp->thread_data->tid : -1);
+    if (interp->parent_interpreter) {
+        fprintf(stderr, ", parent: 0x%lx, tid: %3d",
+                (unsigned long)interp->parent_interpreter,
+                interp->thread_data ? (signed)interp->thread_data->tid : -1);
+    }
 #endif
     fprintf(stderr, "\n");
 

src/gc/gc_inf.c

@@ -444,6 +444,7 @@ Parrot_gc_inf_init(PARROT_INTERP, SHIM(Parrot_GC_Init_Args *args))
     interp->gc_sys->do_gc_mark         = gc_inf_mark_and_sweep;
     interp->gc_sys->finalize_gc_system = NULL;
 
+    interp->gc_sys->maybe_gc_mark           = gc_inf_mark_and_sweep; /* dummy */
     interp->gc_sys->do_gc_mark              = gc_inf_mark_and_sweep;
     interp->gc_sys->compact_string_pool     = gc_inf_compact_memory_pool;
 

src/gc/gc_ms.c

@@ -176,6 +176,9 @@ static void gc_ms_mark_special(PARROT_INTERP, ARGIN(PMC *pmc))
 static void gc_ms_mark_str_header(PARROT_INTERP, ARGMOD_NULLOK(STRING *obj))
         FUNC_MODIFIES(*obj);
 
+static void gc_ms_maybe_mark_and_sweep(PARROT_INTERP, UINTVAL flags)
+        __attribute__nonnull__(1);
+
 static void gc_ms_more_traceable_objects(PARROT_INTERP,
     Memory_Pools *mem_pools,
     ARGMOD(Fixed_Size_Pool *pool))
@@ -226,6 +229,7 @@ static void gc_ms_unblock_GC_mark(PARROT_INTERP)
 static void gc_ms_unblock_GC_sweep(PARROT_INTERP)
         __attribute__nonnull__(1);
 
+PARROT_INLINE
 static void Parrot_gc_allocate_new_attributes_arena(
     ARGMOD(PMC_Attribute_Pool *pool))
         __attribute__nonnull__(1)
@@ -332,6 +336,8 @@ static void Parrot_gc_initialize_fixed_size_pools(PARROT_INTERP,
        PARROT_ASSERT_ARG(interp) \
     , PARROT_ASSERT_ARG(pmc))
 #define ASSERT_ARGS_gc_ms_mark_str_header __attribute__unused__ int _ASSERT_ARGS_CHECK = (0)
+#define ASSERT_ARGS_gc_ms_maybe_mark_and_sweep __attribute__unused__ int _ASSERT_ARGS_CHECK = (\
+       PARROT_ASSERT_ARG(interp))
 #define ASSERT_ARGS_gc_ms_more_traceable_objects __attribute__unused__ int _ASSERT_ARGS_CHECK = (\
        PARROT_ASSERT_ARG(interp) \
     , PARROT_ASSERT_ARG(pool))
@@ -402,6 +408,7 @@ Parrot_gc_ms_init(PARROT_INTERP, SHIM(Parrot_GC_Init_Args *args))
     interp->gc_sys->destroy_child_interp    = gc_ms_destroy_child_interp;
     interp->gc_sys->init_pool               = gc_ms_pool_init;
 
+    interp->gc_sys->maybe_gc_mark           = gc_ms_maybe_mark_and_sweep;
     interp->gc_sys->do_gc_mark              = gc_ms_mark_and_sweep;
     interp->gc_sys->compact_string_pool     = gc_ms_compact_memory_pool;
 
@@ -557,6 +564,28 @@ Parrot_gc_ms_needed(PARROT_INTERP)
 
 /*
 
+=item C<static void gc_ms_maybe_mark_and_sweep(PARROT_INTERP, UINTVAL flags)>
+
+Run a GC if memory used is above threshold.
+
+=cut
+
+*/
+
+static void
+gc_ms_maybe_mark_and_sweep(PARROT_INTERP, UINTVAL flags)
+{
+    ASSERT_ARGS(gc_ms_maybe_mark_and_sweep)
+    struct GC_Subsystem * const gc_sys = interp->gc_sys;
+    Memory_Pools * const self = (Memory_Pools *)gc_sys->gc_private;
+
+    if (!self->gc_mark_block_level
+    &&  Parrot_gc_ms_needed(interp))
+        gc_sys->do_gc_mark(interp, flags);
+}
+
+/*
+
 =item C<static void gc_ms_mark_and_sweep(PARROT_INTERP, UINTVAL flags)>
 
 Runs the stop-the-world mark & sweep (MS) collector.
@@ -1071,11 +1100,11 @@ Parrot_gc_get_attributes_from_pool(PARROT_INTERP, ARGMOD(PMC_Attribute_Pool * po
         return Parrot_gc_get_attributes_from_pool(interp, pool);
     }
 
-    --pool->num_free_objects;
+    --pool->num_free_objects; /* the hottest op in --gc ms: 11.64% */
     return (void *)item;
 }
 
-
+PARROT_INLINE
 static void
 Parrot_gc_allocate_new_attributes_arena(ARGMOD(PMC_Attribute_Pool *pool))
 {

src/gc/gc_ms2.c

@@ -616,8 +616,9 @@ Parrot_gc_ms2_init(PARROT_INTERP, ARGIN(Parrot_GC_Init_Args *args))
      * in threaded parrot */
     gc_sys->finalize_gc_system          = gc_ms2_finalize;
 
-    interp->gc_sys->do_gc_mark                  = gc_ms2_mark_and_sweep;
-    interp->gc_sys->compact_string_pool         = gc_ms2_compact_memory_pool;
+    gc_sys->maybe_gc_mark               = gc_ms2_maybe_mark_and_sweep;
+    gc_sys->do_gc_mark                  = gc_ms2_mark_and_sweep;
+    gc_sys->compact_string_pool         = gc_ms2_compact_memory_pool;
 
     /*
     gc_sys->mark_special                = gc_ms2_mark_special;

src/gc/gc_private.h

@@ -199,6 +199,7 @@ typedef struct GC_Subsystem {
     void (*finalize_gc_system) (PARROT_INTERP);
     void (*destroy_child_interp)(ARGMOD(Interp *dest_interp), ARGIN(Interp *child_interp));
 
+    void (*maybe_gc_mark)(PARROT_INTERP, UINTVAL flags);
     void (*do_gc_mark)(PARROT_INTERP, UINTVAL flags);
     void (*compact_string_pool)(PARROT_INTERP);
 

src/gc/string_gc.c

@@ -630,7 +630,7 @@ mem_allocate(PARROT_INTERP,
     /* If not enough room, try to find some */
     if (pool->top_block->free < size) {
         /* Run a GC if needed */
-        Parrot_gc_maybe_mark_and_sweep(interp, GC_trace_stack_FLAG);
+        interp->gc_sys->maybe_gc_mark(interp, GC_trace_stack_FLAG);
 
         if (pool->top_block->free < size) {
             if (pool->minimum_block_size < 65536 * 16)

src/string/encoding/shared.c

@@ -1098,6 +1098,7 @@ fixed_substr(PARROT_INTERP, ARGIN(const STRING *src), INTVAL offset, INTVAL leng
         return return_string;
 
     bytes_per_codepoint = src->encoding->max_bytes_per_codepoint;
+    PARROT_ASSERT(bytes_per_codepoint <= 4);
     maxlen              = strlen - offset;
 
     if ((UINTVAL)length > maxlen)

t/stress/threads.t

@@ -49,7 +49,7 @@ use Parrot::Config;
 {
     local $ENV{TEST_PROG_ARGS} .= '-t11 --gc-nursery-size=0.001 --gc-debug ';
 
-    pir_exit_code_is( << 'CODE', 0, "IO Stress with -t", todo => 'GH875 threads and -t1: gc_gms_mark_pmc_header: self->work_list might be empty' );
+    pir_exit_code_is( << 'CODE', 0, "IO Stress with -t" );
 .sub test :main
     load_bytecode "dumper.pbc"
     load_bytecode 'Test/More.pbc'