Cleaning up full_sweep_reasons function and making it stock-specific;…

… Adding _Atomic() to mmtk's tls malloc counter
JuliaLang · Dec 10, 2024 · 167c10f · 167c10f
1 parent 4ea1782
commit 167c10f
Show file tree

Hide file tree

Showing 6 changed files with 31 additions and 34 deletions.
diff --git a/Make.inc b/Make.inc
@@ -80,9 +80,6 @@ HAVE_SSP := 0
 WITH_GC_VERIFY := 0
 WITH_GC_DEBUG_ENV := 0
 
-# Use MMTk GC
-WITH_MMTK ?= 0
-
 # Enable DTrace support
 WITH_DTRACE := 0
 
@@ -832,7 +829,7 @@ JCXXFLAGS += -DGC_DEBUG_ENV
 JCFLAGS += -DGC_DEBUG_ENV
 endif
 
-ifeq ($(WITH_MMTK), 1)
+ifneq ($(MMTK_PLAN), )
 ifeq (${MMTK_JULIA_DIR},)
 $(error MMTK_JULIA_DIR must be set to use MMTk)
 endif
@@ -848,10 +845,11 @@ endif
 ifeq (${MMTK_PLAN},Immix)
 JCXXFLAGS += -DMMTK_PLAN_IMMIX
 JCFLAGS += -DMMTK_PLAN_IMMIX
-endif
-ifeq (${MMTK_PLAN},StickyImmix)
+else ifeq (${MMTK_PLAN},StickyImmix)
 JCXXFLAGS += -DMMTK_PLAN_STICKYIMMIX
 JCFLAGS += -DMMTK_PLAN_STICKYIMMIX
+else
+$(error "Unsupported MMTk plan: $(MMTK_PLAN)")
 endif
 MMTK_DIR = ${MMTK_JULIA_DIR}/mmtk
 MMTK_API_INC = $(MMTK_DIR)/api
@@ -1863,7 +1861,7 @@ PRINT_PERL = printf '    %b %b\n' $(PERLCOLOR)PERL$(ENDCOLOR) $(BINCOLOR)$(GOAL)
 PRINT_FLISP = printf '    %b %b\n' $(FLISPCOLOR)FLISP$(ENDCOLOR) $(BINCOLOR)$(GOAL)$(ENDCOLOR); $(1)
 PRINT_JULIA = printf '    %b %b\n' $(JULIACOLOR)JULIA$(ENDCOLOR) $(BINCOLOR)$(GOAL)$(ENDCOLOR); $(1)
 PRINT_DTRACE = printf '    %b %b\n' $(DTRACECOLOR)DTRACE$(ENDCOLOR) $(BINCOLOR)$(GOAL)$(ENDCOLOR); $(1)
-ifeq ($(WITH_MMTK), 1)
+ifneq ($(MMTK_PLAN), )
 PRINT_MMTK = printf '    %b %b\n' $(LINKCOLOR)MMTK$(ENDCOLOR) $(BINCOLOR)$(GOAL)$(ENDCOLOR); $(1)
 endif
 
@@ -1876,7 +1874,7 @@ PRINT_PERL = echo '$(subst ','\'',$(1))'; $(1)
 PRINT_FLISP = echo '$(subst ','\'',$(1))'; $(1)
 PRINT_JULIA = echo '$(subst ','\'',$(1))'; $(1)
 PRINT_DTRACE = echo '$(subst ','\'',$(1))'; $(1)
-ifeq ($(WITH_MMTK), 1)
+ifneq ($(MMTK_PLAN), )
 PRINT_MMTK = echo '$(subst ','\'',$(1))'; $(1)
 endif
 

diff --git a/base/timing.jl b/base/timing.jl
@@ -106,9 +106,12 @@ function gc_page_utilization_data()
     return Base.unsafe_wrap(Array, page_utilization_raw, JL_GC_N_MAX_POOLS, own=false)
 end
 
+# Full sweep reasons are currently only available for the stock GC
+@static if occursin("stock", unsafe_string(ccall(:jl_gc_active_impl, Ptr{UInt8}, ())))
 # must be kept in sync with `src/gc-stock.h``
 const FULL_SWEEP_REASONS = [:FULL_SWEEP_REASON_SWEEP_ALWAYS_FULL, :FULL_SWEEP_REASON_FORCED_FULL_SWEEP,
                             :FULL_SWEEP_REASON_USER_MAX_EXCEEDED, :FULL_SWEEP_REASON_LARGE_PROMOTION_RATE]
+end
 
 """
     Base.full_sweep_reasons()
@@ -124,11 +127,15 @@ The reasons are:
 Note that the set of reasons is not guaranteed to be stable across minor versions of Julia.
 """
 function full_sweep_reasons()
-    reason = cglobal(:jl_full_sweep_reasons, UInt64)
-    reasons_as_array = Base.unsafe_wrap(Vector{UInt64}, reason, length(FULL_SWEEP_REASONS), own=false)
     d = Dict{Symbol, Int64}()
-    for (i, r) in enumerate(FULL_SWEEP_REASONS)
-        d[r] = reasons_as_array[i]
+    # populate the dictionary according to the reasons above for the stock GC
+    # otherwise return an empty dictionary for now
+    @static if occursin("stock", unsafe_string(ccall(:jl_gc_active_impl, Ptr{UInt8}, ())))
+        reason = cglobal(:jl_full_sweep_reasons, UInt64)
+        reasons_as_array = Base.unsafe_wrap(Vector{UInt64}, reason, length(FULL_SWEEP_REASONS), own=false)
+        for (i, r) in enumerate(FULL_SWEEP_REASONS)
+            d[r] = reasons_as_array[i]
+        end
     end
     return d
 end

diff --git a/src/Makefile b/src/Makefile
@@ -46,7 +46,7 @@ endif
 
 # GC source code. It depends on which GC implementation to use.
 GC_SRCS := gc-common gc-stacks gc-alloc-profiler gc-heap-snapshot
-ifeq ($(WITH_MMTK), 1)
+ifneq ($(MMTK_PLAN), )
 GC_SRCS += gc-mmtk
 else
 GC_SRCS += gc-stock gc-debug gc-pages gc-page-profiler
@@ -67,7 +67,7 @@ CG_LLVMLINK :=
 ifeq ($(JULIACODEGEN),LLVM)
 # Currently these files are used by both GCs. But we should make the list specific to stock, and MMTk should have its own implementation.
 GC_CODEGEN_SRCS := llvm-final-gc-lowering llvm-late-gc-lowering llvm-gc-invariant-verifier
-ifeq ($(WITH_MMTK), 1)
+ifneq ($(MMTK_PLAN), )
 FLAGS += -I$(MMTK_API_INC)
 GC_CODEGEN_SRCS += llvm-late-gc-lowering-mmtk
 else
@@ -122,7 +122,7 @@ UV_HEADERS += uv.h
 UV_HEADERS += uv/*.h
 endif
 PUBLIC_HEADERS := $(BUILDDIR)/julia_version.h $(wildcard $(SRCDIR)/support/*.h) $(addprefix $(SRCDIR)/,work-stealing-queue.h gc-interface.h gc-tls-common.h julia.h julia_assert.h julia_threads.h julia_fasttls.h julia_locks.h julia_atomics.h jloptions.h)
-ifeq ($(WITH_MMTK), 1)
+ifneq ($(MMTK_PLAN), )
 	PUBLIC_HEADERS += $(addprefix $(SRCDIR)/,gc-tls-mmtk.h)
 else
 	PUBLIC_HEADERS += $(addprefix $(SRCDIR)/,gc-tls-stock.h)
@@ -250,7 +250,7 @@ $(BUILDDIR)/%.h.gen : $(SRCDIR)/%.d
 	mv $@.tmp $@
 
 # Compile files from the binding side and copy so file into lib folder
-ifeq ($(WITH_MMTK), 1)
+ifneq ($(MMTK_PLAN), )
 $(MMTK_LIB_DST): $(MMTK_LIB_SRC)
 	@$(call PRINT_MMTK, cp $< $@)
 endif

diff --git a/src/gc-interface.h b/src/gc-interface.h
@@ -101,8 +101,8 @@ JL_DLLEXPORT int gc_is_collector_thread(int tid) JL_NOTSAFEPOINT;
 // Returns which GC implementation is being used and possibly its version according to the list of supported GCs
 // NB: it should clearly identify the GC by including e.g. ‘stock’ or ‘mmtk’ as a substring.
 JL_DLLEXPORT const char* jl_gc_active_impl(void);
-// Sweep Julia's stack pools and mtarray buffers. Note that this function has been added to the interface since
-// each GC should implement this but this function will most likely not be used by other code in the runtime.
+// Sweep Julia's stack pools and mtarray buffers. Note that this function has been added to the interface as
+// each GC should implement it but it will most likely not be used by other code in the runtime.
 // It still needs to be annotated with JL_DLLEXPORT since it is called from Rust by MMTk.
 JL_DLLEXPORT void jl_gc_sweep_stack_pools_and_mtarraylist_buffers(jl_ptls_t ptls) JL_NOTSAFEPOINT;
 
@@ -145,7 +145,6 @@ JL_DLLEXPORT uint64_t jl_gc_total_hrtime(void);
 // **must** also set the type of the returning object to be `ty`. The type `ty` may also be used to record
 // an allocation of that type in the allocation profiler.
 struct _jl_value_t *jl_gc_alloc_(struct _jl_tls_states_t * ptls, size_t sz, void *ty);
-
 // Allocates small objects and increments Julia allocation counterst. Size of the object
 // header must be included in the object size. The (possibly unused in some implementations)
 // offset to the arena in which we're allocating is passed in the second parameter, and the
@@ -205,7 +204,6 @@ JL_DLLEXPORT void *jl_gc_perm_alloc(size_t sz, int zero, unsigned align,
 //              the allocated object. All objects stored in fields of this object
 //              must be either permanently allocated or have other roots.
 struct _jl_value_t *jl_gc_permobj(size_t sz, void *ty) JL_NOTSAFEPOINT;
-
 // This function notifies the GC about memory addresses that are set when loading the boot image.
 // The GC may use that information to, for instance, determine that such objects should
 // be treated as marked and belonged to the old generation in nursery collections.

diff --git a/src/gc-mmtk.c b/src/gc-mmtk.c
@@ -1,4 +1,5 @@
 #include "gc-common.h"
+#include "gc-tls-mmtk.h"
 #include "mmtkMutator.h"
 #include "threading.h"
 
@@ -17,19 +18,6 @@ extern jl_value_t *cmpswap_names JL_GLOBALLY_ROOTED;
 extern const unsigned pool_sizes[];
 extern jl_mutex_t finalizers_lock;
 
-// FIXME: Does it make sense for MMTk to implement something similar
-// for now, just ignoring this.
-
-// Must be kept in sync with `base/timing.jl`
-#define FULL_SWEEP_REASON_SWEEP_ALWAYS_FULL (0)
-#define FULL_SWEEP_REASON_FORCED_FULL_SWEEP (1)
-#define FULL_SWEEP_REASON_USER_MAX_EXCEEDED (2)
-#define FULL_SWEEP_REASON_LARGE_PROMOTION_RATE (3)
-#define FULL_SWEEP_NUM_REASONS (4)
-
-// Table recording number of full GCs due to each reason
-JL_DLLEXPORT uint64_t jl_full_sweep_reasons[FULL_SWEEP_NUM_REASONS];
-
 // FIXME: Should the values below be shared between both GC's?
 // Note that MMTk uses a hard max heap limit, which is set by default
 // as 70% of the free available memory. The min heap is set as the

diff --git a/src/gc-tls-mmtk.h b/src/gc-tls-mmtk.h
@@ -1,17 +1,23 @@
 // This file is a part of Julia. License is MIT: https://julialang.org/license
 
+#ifndef JL_GC_TLS_H
+#define JL_GC_TLS_H
+
 #include <assert.h>
 #include "mmtkMutator.h"
+#include "julia_atomics.h"
 
 #ifdef __cplusplus
 extern "C" {
 #endif
 
 typedef struct {
     MMTkMutatorContext mmtk_mutator;
-    size_t malloc_sz_since_last_poll;
+    _Atomic(size_t) malloc_sz_since_last_poll;
 } jl_gc_tls_states_t;
 
 #ifdef __cplusplus
 }
 #endif
+
+#endif // JL_GC_TLS_H