Merge pull request #1664 from clasp-developers/boehm-cleanup

Upgrades to boehm gc 8.2.8 and cleans up some old code. More importantly, it enables the use of thread-local allocation, which should make consing more efficient in a multi threaded program.

Author: Bike

include/clasp/gctools/boehm_config.h

@@ -86,7 +86,7 @@
 #define GC_VERSION_MAJOR 8
 
 /* The micro version number of this GC release. */
-#define GC_VERSION_MICRO 0
+#define GC_VERSION_MICRO 8
 
 /* The minor version number of this GC release. */
 #define GC_VERSION_MINOR 2
@@ -193,7 +193,7 @@
 #define PACKAGE_NAME "gc"
 
 /* Define to the full name and version of this package. */
-#define PACKAGE_STRING "gc 8.2.0"
+#define PACKAGE_STRING "gc 8.2.8"
 
 /* Define to the one symbol short name of this package. */
 #define PACKAGE_TARNAME "gc"
@@ -202,7 +202,7 @@
 #define PACKAGE_URL ""
 
 /* Define to the version of this package. */
-#define PACKAGE_VERSION "8.2.0"
+#define PACKAGE_VERSION "8.2.8"
 
 /* Define to enable parallel marking. */
 /* #undef PARALLEL_MARK */
@@ -256,7 +256,7 @@
 /* #undef USE_WINALLOC */
 
 /* Version number of package */
-#define VERSION "8.2.0"
+#define VERSION "8.2.8"
 
 /* The POSIX feature macro. */
 /* #undef _POSIX_C_SOURCE */

include/clasp/gctools/gc_boot.h

@@ -153,18 +153,21 @@ struct Container_layout {
 
 enum Layout_operation { class_container_op, bitunit_container_op, templated_op, undefined_op };
 
+#ifdef USE_BOEHM
 #define KIND_UNDEFINED 99999
 struct Boehm_info {
   bool _kind_defined = false;
-  int _container_element_work = 0;
   uintptr_t _kind = KIND_UNDEFINED;
 };
+#endif // USE_BOEHM
 
 struct Stamp_layout {
   // One of class_container_op, bitunit_container_op, templated_op
   Layout_operation layout_op = undefined_op;
   const char* name;
+#ifdef USE_BOEHM
   Boehm_info boehm;
+#endif // USE_BOEHM
   // A bitmap of pointer fields for mps fixing and (once shifted right to skip clasp header - boehm marking)
   // The most significant bit indicates the vtable - it must be zero
   uintptr_t class_field_pointer_bitmap = 0;

include/clasp/gctools/gcalloc_boehm.h

@@ -4,11 +4,11 @@
 #define ALIGNED_GC_MALLOC(sz) MAYBE_MONITOR_ALLOC(GC_MALLOC(sz), sz)
 #define ALIGNED_GC_MALLOC_ATOMIC(sz) MAYBE_MONITOR_ALLOC(GC_MALLOC_ATOMIC(sz), sz)
 #define ALIGNED_GC_MALLOC_UNCOLLECTABLE(sz) MAYBE_MONITOR_ALLOC(GC_MALLOC_UNCOLLECTABLE(sz), sz)
-#define ALIGNED_GC_MALLOC_KIND(sz, knd) MAYBE_MONITOR_ALLOC(GC_malloc_kind_global(sz, knd), sz)
-#define ALIGNED_GC_MALLOC_STRONG_WEAK_KIND(sz, knd) MAYBE_MONITOR_ALLOC(GC_malloc_kind_global(sz, knd), sz)
+#define ALIGNED_GC_MALLOC_KIND(sz, knd) MAYBE_MONITOR_ALLOC(GC_malloc_kind(sz, knd), sz)
+#define ALIGNED_GC_MALLOC_STRONG_WEAK_KIND(sz, knd) MAYBE_MONITOR_ALLOC(GC_malloc_kind(sz, knd), sz)
 #define ALIGNED_GC_MALLOC_ATOMIC_KIND(stmp, sz, knd, kndaddr)                                                                      \
   MAYBE_MONITOR_ALLOC(                                                                                                             \
-      (knd == GC_I_PTRFREE) ? GC_malloc_kind_global(sz, knd) : malloc_kind_error(GC_I_PTRFREE, knd, sz, stmp, kndaddr), sz)
+      (knd == GC_I_PTRFREE) ? GC_malloc_kind(sz, knd) : malloc_kind_error(GC_I_PTRFREE, knd, sz, stmp, kndaddr), sz)
 #define ALIGNED_GC_MALLOC_UNCOLLECTABLE_KIND(sz, knd)                                                                              \
   MAYBE_MONITOR_ALLOC(GC_generic_malloc_uncollectable(sz, knd), sz)
 #else

include/clasp/gctools/gcweak.h

@@ -89,11 +89,8 @@ struct WeakPointer {
   void store_no_lock(core::T_sp); // ditto.
   void store(core::T_sp);
   void fixupInternalsForSnapshotSaveLoad(snapshotSaveLoad::Fixup*);
-public: // has to be public for precise GC reasons even though it's not scanned?
-  // This is a Tagged rather than a T_sp because something in gc_boot seems to
-  // check for T_sps in atomic (pointerless) objects. Rather than lie harder we
-  // can just do this and build a T_sp from it as required.
-  Tagged _value;
+private:
+  core::T_sp _value;
 #ifdef USE_BOEHM
   // flag needed to disambiguate fixnum 0 from splatted pointer
   bool _splattablep = false;

include/clasp/gctools/memoryManagement.h

@@ -483,9 +483,6 @@ class BaseHeader_s {
     void* fwdPointer() const { return reinterpret_cast<void*>(this->_header_data[0] & (~(uintptr_t)mtag_mask)); };
     /*! Return the size of the fwd block - without the header. This reaches into the client area to get the size */
     void setFwdPointer(void* ptr) { this->_header_data[0] = reinterpret_cast<uintptr_t>(ptr) | fwd_mtag; };
-    uintptr_t fwdSize() const { return this->_header_data[1]; };
-    /*! This writes into the first tagged_stamp_t sized word of the client data. */
-    void setFwdSize(size_t sz) { this->_header_data[1] = sz; };
     /*! Define the header as a pad, pass pad_tag or pad1_tag */
     void setPad(tagged_stamp_t p) { this->_header_data[0] = p; };
     /*! Return the pad1 size */
@@ -610,7 +607,7 @@ class BaseHeader_s {
       return "Header_CONS";
     } else if (this->_badge_stamp_wtag_mtag.fwdP()) {
       std::stringstream ss;
-      ss << "Fwd/ptr=" << this->_badge_stamp_wtag_mtag.fwdPointer() << "/sz=" << this->_badge_stamp_wtag_mtag.fwdSize();
+      ss << "Fwd/ptr=" << this->_badge_stamp_wtag_mtag.fwdPointer();
       return ss.str();
     } else if (this->_badge_stamp_wtag_mtag.pad1P()) {
       return "Pad1";

repos.sexp

@@ -207,7 +207,7 @@
  (:name :bdwgc
   :repository "https://github.com/ivmai/bdwgc.git"
   :directory "src/bdwgc/"
-  :commit "v8.2.0")
+  :commit "v8.2.8")
  (:name :libatomic_ops
   :repository "https://github.com/ivmai/libatomic_ops.git"
   :directory "src/libatomic_ops/"

src/gctools/cons_scan.cc

@@ -10,7 +10,6 @@
 #define POINTER_FIX(field)         // Macro to fix pointer at field
 #define CONS_SCAN                  // Macro to turn on #ifdef inclusion of code
 #define CONS_SKIP                  // Macro to turn on #ifdef inclusion of code
-#define CONS_FWD                   // Macro to turn on #ifdef inclusion of code
 #define EXTRA_ARGUMENTS            // Add extra arguments
  */
 
@@ -59,19 +58,3 @@ ADDR_T CONS_SKIP(ADDR_T client, size_t& objectSize) {
   return client;
 }
 #endif // CONS_SKIP
-
-#ifdef CONS_FWD
-#ifdef CONS_SKIP_IN_CONS_FWD
-ADDR_T CONS_SKIP_IN_CONS_FWD(ADDR_T client);
-#endif // CONS_SKIP_IN_CONS_FWD
-
-static void CONS_FWD(ADDR_T old_client, ADDR_T new_client) {
-  //  printf("%s:%d in %s\n", __FILE__, __LINE__, __FUNCTION__ );
-  // I'm assuming both old and new client pointers have valid headers at this point
-  CONS_SKIP_IN_CONS_FWD(old_client);
-  core::Cons_O* cons = reinterpret_cast<core::Cons_O*>(old_client);
-  gctools::Header_s* header = (gctools::Header_s*)gctools::ConsPtrToHeaderPtr(cons);
-  header->_badge_stamp_wtag_mtag.setFwdPointer((void*)new_client);
-  header->_badge_stamp_wtag_mtag.setFwdSize(sizeof(core::Cons_O));
-}
-#endif // CONS_FWD

src/gctools/gc_boot.cc

@@ -20,15 +20,6 @@
 #define DGC_PRINT(...)
 #endif
 
-#if defined(USE_BOEHM) && defined(USE_PRECISE_GC)
-extern "C" {
-void* obj_skip(void*);
-};
-#define GC_LISP_OBJECT_MARK
-#include "obj_scan.cc"
-#undef GC_LISP_OBJECT_MARK
-#endif
-
 namespace gctools {
 
 uintptr_t global_lisp_kind;
@@ -400,17 +391,12 @@ void walk_stamp_field_layout_tables(WalkKind walk, std::ostream& fout) {
     }
 
     // Use boehm in the precise GC mode
-    //  global_container_proc_index = GC_new_proc_inner((GC_mark_proc)class_container_mark);
-    global_lisp_kind = GC_new_kind(GC_new_free_list(), GC_DS_LENGTH, 1, 1);
-    global_cons_kind = GC_new_kind(GC_new_free_list(), GC_DS_LENGTH, 1, 1);
-    global_class_kind =
-        GC_new_kind(GC_new_free_list(), GC_DS_LENGTH, 1,
-                    1); //  GC_MAKE_PROC(GC_new_proc((GC_mark_proc)Lisp_object_mark),0), 0, 1); // GC_DS_LENGTH, 1, 1);
-    global_container_kind = GC_new_kind(
-        GC_new_free_list(), GC_DS_LENGTH, 1,
-        1); // */  GC_new_kind(GC_new_free_list(), GC_MAKE_PROC(global_container_proc_index,0),0,1); // GC_DS_LENGTH, 1, 1);
-    global_code_kind = GC_new_kind(GC_new_free_list(), GC_DS_LENGTH, 1, 1);
-    global_atomic_kind = GC_I_PTRFREE; // GC_new_kind(GC_new_free_list(), GC_DS_LENGTH, 0, 1);
+    global_lisp_kind = GC_I_NORMAL;
+    global_cons_kind = GC_I_NORMAL;
+    global_class_kind = GC_I_NORMAL;
+    global_container_kind = GC_I_NORMAL;
+    global_code_kind = GC_I_NORMAL;
+    global_atomic_kind = GC_I_PTRFREE;
     for (cur_stamp = 0; cur_stamp <= local_stamp_max; ++cur_stamp) {
       if (local_stamp_layout[cur_stamp].layout_op != undefined_op) {
 #ifdef DUMP_PRECISE_CALC
@@ -461,10 +447,6 @@ void walk_stamp_field_layout_tables(WalkKind walk, std::ostream& fout) {
                   "%s:%d WARNING There are too many pointers (%d) in each element of a container to break up the work for boehm\n",
                   __FILE__, __LINE__, pointer_count);
             }
-            // Calculate the number of elements worth of pointers are processed with each
-            // call to the marking procedure
-            int container_element_work = pointer_count ? (GC_PROC_BYTES / 8 / 2) / pointer_count : 0;
-            local_stamp_layout[cur_stamp].boehm._container_element_work = container_element_work;
             if (class_bitmap && !container_bitmap) {
               // There are no pointers in the container part
               // - so we can use the bitmap_skip_header
@@ -512,11 +494,13 @@ void walk_stamp_field_layout_tables(WalkKind walk, std::ostream& fout) {
   } else if (walk == precise_info) {
     // Check that everything is ok
     if (getenv("CLASP_DEBUG_STAMP_INFO")) {
+#if defined(USE_BOEHM) && defined(USE_PRECISE_GC)
       for (size_t stamp = 0; stamp <= local_stamp_max; stamp++) {
         printf("%s:%d:%s stamp: %3lu  boehm._kind_defined %2d  boehm._kind %5lu  name: %s\n", __FILE__, __LINE__, __FUNCTION__,
                stamp, local_stamp_layout[stamp].boehm._kind_defined, local_stamp_layout[stamp].boehm._kind,
                local_stamp_layout[stamp].name);
       }
+#endif // defined(USE_BOEHM) && defined(USE_PRECISE_GC)
       printf("%s:%d:%s local_stamp_max: %lu\n", __FILE__, __LINE__, __FUNCTION__, local_stamp_max);
     }
     global_stamp_max = local_stamp_max;

src/gctools/gcweak.cc

@@ -34,7 +34,7 @@ THE SOFTWARE.
 namespace gctools {
 
 #ifdef USE_BOEHM
-WeakPointer::WeakPointer(core::T_sp o) : _value(o.tagged_()) {
+WeakPointer::WeakPointer(core::T_sp o) : _value(o) {
   if (o.objectp()) { // pointer, so we're actually weak
     _splattablep = true;
     // note: deregistered automatically if the weak pointer itself is dealloc'd
@@ -45,8 +45,8 @@ WeakPointer::WeakPointer(core::T_sp o) : _value(o.tagged_()) {
 void* WeakPointer::value_helper(void* data) {
   value_helper_s* vhsp = (value_helper_s*)data;
   if (vhsp->wp->_value || !vhsp->wp->_splattablep) // not splatted
-    // construct a T_sp in the result
-    vhsp->result.emplace(vhsp->wp->_value);
+    // put a T_sp in the result
+    vhsp->result = vhsp->wp->_value;
   // otherwise, leave the result default constructed (no T_sp)
   return nullptr; // unused
 }
@@ -59,11 +59,11 @@ std::optional<core::T_sp> WeakPointer::value() const {
 }
 std::optional<core::T_sp> WeakPointer::value_no_lock() const {
   if (_value || !_splattablep)
-    return core::T_sp(_value);
+    return _value;
   else return std::nullopt;
 }
 void WeakPointer::store_no_lock(core::T_sp o) {
-  _value = o.tagged_();
+  _value = o;
   _splattablep = o.objectp();
   // links set up in fixupInternals below.
 }
@@ -76,31 +76,30 @@ void WeakPointer::store(core::T_sp o) {
     _splattablep = false;
     GC_unregister_disappearing_link((void**)&_value);
   }
-  _value = o.tagged_();
+  _value = o;
 }  
 
 void WeakPointer::fixupInternalsForSnapshotSaveLoad(snapshotSaveLoad::Fixup* fixup) {
   if (snapshotSaveLoad::operation(fixup) == snapshotSaveLoad::LoadOp) {
     // We do this later rather than in store_no_lock because register/deregister
     // unconditionally grab a lock. This is a bit of a KLUDGE.
-    core::T_sp o(_value);
-    if (o.objectp()) {
+    if (_value.objectp()) {
       _splattablep = true;
       // Implicitly removes any previous registration, per boehm docs.
-      GC_general_register_disappearing_link((void**)&_value, &*o);
+      GC_general_register_disappearing_link((void**)&_value, &*_value);
     } else {
       _splattablep = false;
       GC_unregister_disappearing_link((void**)&_value);
     }
   }
 }
 #else // not-actually-weak pointers - TODO for your other GC!
-WeakPointer::WeakPointer(core::T_sp o) : _value(o.tagged_()) {}
+WeakPointer::WeakPointer(core::T_sp o) : _value(o) {}
 
 // always valid
-std::optional<core::T_sp> WeakPointer::value() const { return core::T_sp(_value); }
+std::optional<core::T_sp> WeakPointer::value() const { return _value; }
 std::optional<core::T_sp> WeakPointer::value_no_lock() const { return value(); }
-void WeakPointer::store_no_lock(core::T_sp o) { _value = o.tagged_(); }
+void WeakPointer::store_no_lock(core::T_sp o) { _value = o; }
 void WeakPointer::store(core::T_sp o) { store_no_lock(o); }
 void WeakPointer::fixupInternalsForSnapshotSaveLoad(snapshotSaveLoad::Fixup*) {}
 #endif

src/gctools/memoryManagement.cc

@@ -400,8 +400,6 @@ void rawHeaderDescribe(const uintptr_t* headerP) {
     Header_s* hdr = (Header_s*)headerP;
     printf("  0x%p : 0x%" PRIuPTR " 0x%" PRIuPTR "\n", headerP, *headerP, *(headerP + 1));
     printf(" fwd_tag - fwd address: 0x%" PRIuPTR "\n", (*headerP) & Header_s::mtag_mask);
-    printf("     fwdSize = %" PRIuPTR "/0x%" PRIuPTR "\n", hdr->_badge_stamp_wtag_mtag.fwdSize(),
-           hdr->_badge_stamp_wtag_mtag.fwdSize());
   } break;
   case Header_s::pad1_mtag:
     printf("  0x%p : 0x%" PRIuPTR " 0x%" PRIuPTR "\n", headerP, *headerP, *(headerP + 1));
@@ -1017,7 +1015,7 @@ size_t objectSize(BaseHeader_s* header) {
     // It's a general object - walk it
     size_t objectSize;
     uintptr_t client = (uintptr_t)HeaderPtrToGeneralPtr<void*>(header);
-    mw_obj_skip(client, false, objectSize);
+    mw_obj_skip(client, objectSize);
     return objectSize;
   }
 }