Updates to the Julia GC integration

lib/init.g

@@ -564,6 +564,11 @@ BindGlobal( "ShowKernelInformation", function()
     fi;
     Add( config, str );
   fi;
+  if IsBound( GAPInfo.KernelInfo.JULIA_VERSION ) then
+    str := "Julia ";
+    Append(str, GAPInfo.KernelInfo.JULIA_VERSION);
+    Add( config, str );
+  fi;
   if IsBound( GAPInfo.UseReadline ) then
     Add( config, "readline" );
   fi;

src/gap.c

@@ -50,6 +50,9 @@
 
 #include <gmp.h>
 
+#ifdef USE_JULIA_GC
+#include "julia.h"
+#endif
 
 static Obj Error;
 
@@ -896,21 +899,28 @@ Obj FuncGASMAN (
                "you can replace <cmd> via 'return <cmd>;'" );
        }
 
-#if !defined(USE_GASMAN)
+        // perform full garbage collection
         if ( strcmp( CSTR_STRING(cmd), "collect" ) == 0 ) {
             CollectBags(0,1);
         }
+
+        // perform partial garbage collection
+        else if ( strcmp( CSTR_STRING(cmd), "partial" ) == 0 ) {
+            CollectBags(0,0);
+        }
+
+#if !defined(USE_GASMAN)
         else {
             cmd = ErrorReturnObj(
-                "GASMAN: <cmd> must be \"collect\"", 0L, 0L,
+                "GASMAN: <cmd> must be \"collect\" or \"partial\"", 0L, 0L,
                 "you can replace <cmd> via 'return <cmd>;'" );
             goto again;
         }
 
 #else
 
         /* if request display the statistics                               */
-        if ( strcmp( CSTR_STRING(cmd), "display" ) == 0 ) {
+        else if ( strcmp( CSTR_STRING(cmd), "display" ) == 0 ) {
 #ifdef COUNT_BAGS
             Pr( "%40s ", (Int)"type",  0L          );
             Pr( "%8s %8s ",  (Int)"alive", (Int)"kbyte" );
@@ -970,16 +980,6 @@ Obj FuncGASMAN (
 #endif
         }
 
-        /* or collect the garbage                                          */
-        else if ( strcmp( CSTR_STRING(cmd), "collect" ) == 0 ) {
-            CollectBags(0,1);
-        }
-
-        /* or collect the garbage                                          */
-        else if ( strcmp( CSTR_STRING(cmd), "partial" ) == 0 ) {
-            CollectBags(0,0);
-        }
-
         /* or display information about global bags                        */
         else if ( strcmp( CSTR_STRING(cmd), "global" ) == 0 ) {
             for ( i = 0;  i < GlobalBags.nr;  i++ ) {
@@ -1438,6 +1438,13 @@ Obj FuncKERNEL_INFO(Obj self) {
   r = RNamName("GMP_VERSION");
   AssPRec(res, r, str);
 
+#ifdef USE_JULIA_GC
+  /* export Julia version  */
+  str = MakeImmString( jl_ver_string() );
+  r = RNamName("JULIA_VERSION");
+  AssPRec(res, r, str);
+#endif
+
   r = RNamName("KernelDebug");
 #ifdef GAP_KERNEL_DEBUG
   AssPRec(res, r, True);

src/gasman.h

@@ -883,42 +883,6 @@ extern void MarkBag( Bag bag );
 extern void MarkArrayOfBags(const Bag array[], UInt count);
 
 
-/****************************************************************************
-**
-*F  InitSweepFuncBags(<type>,<sweep-func>)  . . . . install sweeping function
-**
-**  'InitSweepFuncBags( <type>, <sweep-func> )'
-**
-**  'InitSweepFuncBags' installs the function <sweep-func> as sweeping
-**  function for bags of type <type>.  
-**
-**  A sweeping function is a function that takes two arguments src and dst of
-**  type Bag *, and  a third, length of type  UInt, and returns nothing. When
-**  it  is called, src points to  the start of the data  area of one bag, and
-**  dst to another. The function should copy the  data from the source bag to
-**  the destination, making any appropriate changes.
-**
-**  Those functions are applied during  the garbage collection to each marked
-**  bag, i.e., bags that are assumed  to be still live  to move them to their
-**  new  position. The  intended  use is  for  weak  pointer bags, which must
-**  remove references to identifiers of  any half-dead objects. 
-**
-**  If no function  is installed for a TNum,  then the data is  simply copied
-**  unchanged and this is done particularly quickly 
-*/
-
-#ifdef USE_GASMAN
-typedef void            (* TNumSweepFuncBags ) (
-            Bag  *               src,
-            Bag *                dst,
-            UInt                 length);
-
-extern  void            InitSweepFuncBags (
-            UInt                tnum,
-            TNumSweepFuncBags    sweep_func );
-#endif
-
-
 /****************************************************************************
 **
 *F  InitGlobalBag(<addr>) . . . . . inform Gasman about global bag identifier

src/gasman_intern.h

@@ -31,6 +31,40 @@ extern  Bag *                   MptrEndBags;
 extern  Bag *                   AllocBags;
 
 
+/****************************************************************************
+**
+*F  InitSweepFuncBags(<type>,<sweep-func>)  . . . . install sweeping function
+**
+**  'InitSweepFuncBags( <type>, <sweep-func> )'
+**
+**  'InitSweepFuncBags' installs the function <sweep-func> as sweeping
+**  function for bags of type <type>.
+**
+**  A sweeping function is a function that takes two arguments src and dst of
+**  type Bag *, and  a third, length of type  UInt, and returns nothing. When
+**  it  is called, src points to  the start of the data  area of one bag, and
+**  dst to another. The function should copy the  data from the source bag to
+**  the destination, making any appropriate changes.
+**
+**  Those functions are applied during  the garbage collection to each marked
+**  bag, i.e., bags that are assumed  to be still live  to move them to their
+**  new  position. The  intended  use is  for  weak  pointer bags, which must
+**  remove references to identifiers of  any half-dead objects.
+**
+**  If no function  is installed for a TNum,  then the data is  simply copied
+**  unchanged and this is done particularly quickly.
+*/
+
+typedef void            (* TNumSweepFuncBags ) (
+            Bag  *               src,
+            Bag *                dst,
+            UInt                 length);
+
+extern  void            InitSweepFuncBags (
+            UInt                tnum,
+            TNumSweepFuncBags    sweep_func );
+
+
 /****************************************************************************
 **
 *V  GlobalBags  . . . . . . . . . . . . . . . . . . . . . list of global bags

src/julia_gc.c

@@ -42,16 +42,19 @@
 // conservative mechanism is used. It should be disabled for precise
 // object tracing, however. The cache does not affect conservative
 // *stack* tracing at all, only conservative tracing of objects.
+//
+// It functions by remembering valid object references in a (lossy)
+// hash table. If we find an object reference in that table, we no
+// longer need to verify that it is accurate, which is a potentially
+// expensive call to the Julia runtime.
 
 static Bag MarkCache[MARK_CACHE_SIZE];
 #ifdef STAT_MARK_CACHE
 static UInt MarkCacheHits, MarkCacheAttempts, MarkCacheCollisions;
 #endif
 
 
-enum { NTYPES = 256 };
-
-TNumInfoBags InfoBags[NTYPES];
+TNumInfoBags InfoBags[NUM_TYPES];
 
 UInt SizeAllBags;
 
@@ -66,7 +69,7 @@ static inline Bag * DATA(BagHeader * bag)
 *F  InitFreeFuncBag(<type>,<free-func>)
 */
 
-TNumFreeFuncBags TabFreeFuncBags[NTYPES];
+TNumFreeFuncBags TabFreeFuncBags[NUM_TYPES];
 
 void InitFreeFuncBag(UInt type, TNumFreeFuncBags finalizer_func)
 {
@@ -79,9 +82,9 @@ void JFinalizer(jl_value_t * obj)
     Bag         contents = (Bag)(hdr + 1);
     UInt        tnum = hdr->type;
 
-    // calling MakeImmutable on a weak pointer object can turn it into a
-    // plist, which no longer requires a free function; but we already
-    // scheduled one, so we must check TabFreeFuncBags here
+    // if a bag needing a finalizer is retyped to a new tnum which no longer
+    // needs one, it may happen that JFinalize is called even though
+    // TabFreeFuncBags[tnum] is NULL
     if (TabFreeFuncBags[tnum])
         TabFreeFuncBags[tnum]((Bag)&contents);
 }
@@ -393,7 +396,7 @@ static void * AllocateBagMemory(UInt type, UInt size)
     return result;
 }
 
-TNumMarkFuncBags TabMarkFuncBags[NTYPES];
+TNumMarkFuncBags TabMarkFuncBags[NUM_TYPES];
 
 void InitMarkFuncBags(UInt type, TNumMarkFuncBags mark_func)
 {
@@ -456,6 +459,8 @@ static void TryMark(void * p)
         }
     }
     else {
+        // Prepopulate the mark cache with references we know
+        // are valid and in current use.
         if (jl_typeis(p2, datatype_mptr))
             MarkCache[MARK_HASH((UInt)p2)] = (Bag)p2;
     }
@@ -573,7 +578,7 @@ static uintptr_t JMarkBag(jl_ptls_t ptls, jl_value_t * obj)
 void InitBags(UInt initial_size, Bag * stack_bottom, UInt stack_align)
 {
     // HOOK: initialization happens here.
-    for (UInt i = 0; i < NTYPES; i++)
+    for (UInt i = 0; i < NUM_TYPES; i++)
         TabMarkFuncBags[i] = MarkAllSubBags;
     // These callbacks need to be set before initialization so
     // that we can track objects allocated during `jl_init()`.
@@ -647,25 +652,26 @@ void RetypeBag(Bag bag, UInt new_type)
 #endif
 
     if (!TabFreeFuncBags[old_type] && TabFreeFuncBags[new_type]) {
-        // calling MakeImmutable on a weak pointer object can turn it into a
-        // plist, which no longer requires a free functions; we allow this,
-        // but we disallow changing the other way. Otherwise, we might end up
-        // switch an object several times between types which require a free
-        // callback and types which don't.
+        // Retyping a bag can change whether a finalizer needs to be run for
+        // it or not, depending on whether TabFreeFuncBags[tnum] is NULL or
+        // not. While JFinalizer checks for this, there is a deeper problem:
+        // jl_gc_schedule_foreign_sweepfunc is not idempotent, and we must not
+        // call it more than once for any given bag. But this could happen if
+        // a bag changed its tnum multiple times between one needing and one
+        // not needing a finalizer. To avoid this, we only allow changing from
+        // needing a finalizer to not needing one, but not the other way
+        // around.
         //
-        // But then we either end up calling jl_gc_schedule_foreign_sweepfunc
-        // multiple times for the same object, which then can lead to Julia
-        // invoking the free function on that object multiple times, which in
-        // turn could have bad consequences.
-        //
-        // Or else we would have to write code to to track whether
-        // jl_gc_schedule_foreign_sweepfunc was called for an object (e.g. by
-        // using an object flag). But right now no GAP code needs to do this,
-        // and changing the type of an object to a completely different type
-        // is something better to be avoided anyway. So instead of supporting
-        // a feature nobody uses right now, we error out and wait to see if
-        // somebody complains.
+        // The alternative would be to write code which tracks whether
+        // jl_gc_schedule_foreign_sweepfunc was already called for an object
+        // (e.g. by using an object flag). But right now no GAP code needs to
+        // do this, and changing the type of an object to a completely
+        // different type is something better to be avoided anyway. So instead
+        // of supporting a feature nobody uses right now, we error out and
+        // wait to see if somebody complains.
         Panic("cannot change bag type to one which requires a 'free' callback");
+
+
     }
     header->type = new_type;
 }
@@ -773,20 +779,28 @@ inline void MarkBag(Bag bag)
     MarkCacheAttempts++;
 #endif
     UInt hash = MARK_HASH((UInt)bag);
-    if (MarkCache[hash] == bag) {
+    if (MarkCache[hash] != bag) {
+        // not in the cache, so verify it explicitly
+        if (jl_gc_internal_obj_base_ptr(p) != p) {
+            // not a valid object
+            return;
+        }
 #ifdef STAT_MARK_CACHE
-        MarkCacheHits++;
+        if (MarkCache[hash])
+            MarkCacheCollisions++;
 #endif
-    }
-    else if (!jl_gc_is_internal_obj_alloc(p) ||
-             !jl_typeis(p, datatype_mptr)) {
-        return;
-    }
+        MarkCache[hash] = bag;
+    } else {
 #ifdef STAT_MARK_CACHE
-    if (MarkCache[hash])
-        MarkCacheCollisions++;
+        MarkCacheHits++;
 #endif
-    MarkCache[hash] = bag;
+    }
+    // The following code is a performance optimization and
+    // relies on Julia internals. It is functionally equivalent
+    // to:
+    //
+    //     if (JMark(p)) YoungRef++;
+    //
     switch (jl_astaggedvalue(p)->bits.gc) {
     case 0:
         if (JMark(p))
@@ -802,16 +816,6 @@ inline void MarkBag(Bag bag)
     }
 }
 
-// 'MarkJuliaRef' is a variant of MarkBag which omits any checks; used by the
-// weak pointer object code to mark jl_weakref_t objects (we can't use MarkBag
-// for that, as it only accepts master pointers; on the up side, we avoid a
-// potential expensive call to jl_gc_is_internal_obj_alloc)
-void MarkJuliaRef(void * p)
-{
-    if (JMark(p))
-        YoungRef++;
-}
-
 inline void MarkArrayOfBags(const Bag array[], UInt count)
 {
     for (UInt i = 0; i < count; i++) {

src/weakptr.c

@@ -70,10 +70,6 @@ static inline void FORGET_WP(Obj wp, UInt pos)
     GC_unregister_disappearing_link(ptr);
 }
 
-#elif defined(USE_JULIA_GC)
-
-extern void MarkJuliaRef(void * p);
-
 #endif
 
 
@@ -593,29 +589,18 @@ Obj FuncIsWPObj( Obj self, Obj wp)
 **  pointers can be reclaimed.  
 */
 
-#if defined(USE_GASMAN) || defined(USE_JULIA_GC)
+#if defined(USE_GASMAN)
 
 static void MarkWeakPointerObj(Obj wp)
 {
     // can't use the stored length here, in case we are in the middle of
     // copying
     const UInt len = SIZE_BAG(wp) / sizeof(Obj) - 1;
     for (UInt i = 1; i <= len; i++) {
-#if defined(USE_JULIA_GC)
-        Obj elm = ADDR_OBJ(wp)[i];
-        if (IS_BAG_REF(elm))
-            MarkJuliaRef(elm);
-#else
         MarkBagWeakly(ADDR_OBJ(wp)[i]);
-#endif
     }
 }
 
-#endif
-
-
-#if defined(USE_GASMAN)
-
 static void SweepWeakPointerObj( Bag *src, Bag *dst, UInt len)
 {
   Bag elm;
@@ -927,9 +912,9 @@ static Int InitKernel (
     InitSweepFuncBags( T_WPOBJ +COPYING, SweepWeakPointerObj  );
   #endif
 #elif defined(USE_JULIA_GC)
-    InitMarkFuncBags ( T_WPOBJ,          MarkWeakPointerObj   );
+    InitMarkFuncBags ( T_WPOBJ,          MarkAllButFirstSubBags   );
   #if !defined(USE_THREADSAFE_COPYING)
-    InitMarkFuncBags ( T_WPOBJ +COPYING, MarkWeakPointerObj   );
+    InitMarkFuncBags ( T_WPOBJ +COPYING, MarkAllSubBags   );
   #endif
 #else
 #error Unknown garbage collector implementation, no weak pointer object implemention available