Remove dependencies from Singleton and FlagLock (#725)

* Move get_tid() to ds_core.

* Rewrite Singleton to only use Aal.

* Move data structure to be usable from the Pal.

src/snmalloc/README.md

@@ -9,6 +9,7 @@ These are arranged in a hierarchy such that each of the directories may include
  - `aal/` provides the architecture abstraction layer (AAL).
    This layer provides abstractions over CPU-specific intrinsics and defines things such as the virtual address-space size.
    There is a single AAL for an snmalloc instantiation.
+ - `ds_aal/` provides data structures that depend on the AAL.
  - `pal/` provides the platform abstraction layer (PAL).
    This exposes OS- or environment-specific abstractions into the rest of the code.
    An snmalloc instantiation may use more than one PAL, including ones provided by the user.

src/snmalloc/ds/ds.h

@@ -3,12 +3,11 @@
  *
  */
 #pragma once
+#include "../ds_aal/ds_aal.h"
 #include "../pal/pal.h"
 #include "aba.h"
 #include "allocconfig.h"
 #include "combininglock.h"
 #include "entropy.h"
-#include "flaglock.h"
 #include "mpmcstack.h"
 #include "pagemap.h"
-#include "singleton.h"

src/snmalloc/ds_aal/ds_aal.h

@@ -0,0 +1,9 @@
+/**
+ * Data structures used by snmalloc that only depend on the AAL (Architecture
+ * Abstraction Layer) and not on the platform. These structures can be used
+ * to implement the Pal.
+ */
+#pragma once
+#include "../aal/aal.h"
+#include "flaglock.h"
+#include "singleton.h"

src/snmalloc/ds_aal/flaglock.h

@@ -1,7 +1,7 @@
 #pragma once
 
 #include "../aal/aal.h"
-#include "../pal/pal.h"
+#include "snmalloc/ds_core/ds_core.h"
 #include "snmalloc/stl/atomic.h"
 
 namespace snmalloc
@@ -13,7 +13,7 @@ namespace snmalloc
    */
   struct DebugFlagWord
   {
-    using ThreadIdentity = DefaultPal::ThreadIdentity;
+    using ThreadIdentity = size_t;
 
     /**
      * @brief flag
@@ -69,7 +69,7 @@ namespace snmalloc
      */
     static ThreadIdentity get_thread_identity()
     {
-      return DefaultPal::get_tid();
+      return debug_get_tid();
     }
   };
 

src/snmalloc/ds_aal/singleton.h

@@ -1,9 +1,6 @@
 #pragma once
 
-#include "../ds_core/ds_core.h"
-#include "flaglock.h"
 #include "snmalloc/stl/atomic.h"
-#include "snmalloc/stl/type_traits.h"
 
 namespace snmalloc
 {
@@ -15,8 +12,14 @@ namespace snmalloc
   template<class Object, void init(Object*) noexcept>
   class Singleton
   {
-    inline static FlagWord flag;
-    inline static stl::Atomic<bool> initialised{false};
+    enum class State
+    {
+      Uninitialised,
+      Initialising,
+      Initialised
+    };
+
+    inline static stl::Atomic<State> initialised{State::Uninitialised};
     inline static Object obj;
 
   public:
@@ -30,20 +33,26 @@ namespace snmalloc
       // If defined should be initially false;
       SNMALLOC_ASSERT(first == nullptr || *first == false);
 
-      if (SNMALLOC_UNLIKELY(!initialised.load(stl::memory_order_acquire)))
+      auto state = initialised.load(stl::memory_order_acquire);
+      if (SNMALLOC_UNLIKELY(state == State::Uninitialised))
       {
-        with(flag, [&]() {
-          if (!initialised)
-          {
-            init(&obj);
-            initialised.store(true, stl::memory_order_release);
-            if (first != nullptr)
-              *first = true;
-          }
-        });
+        if (initialised.compare_exchange_strong(
+              state, State::Initialising, stl::memory_order_relaxed))
+        {
+          init(&obj);
+          initialised.store(State::Initialised, stl::memory_order_release);
+          if (first != nullptr)
+            *first = true;
+        }
       }
+
+      while (SNMALLOC_UNLIKELY(state != State::Initialised))
+      {
+        Aal::pause();
+        state = initialised.load(stl::memory_order_acquire);
+      }
+
       return obj;
     }
   };
-
 } // namespace snmalloc

src/snmalloc/ds_core/ds_core.h

@@ -15,3 +15,4 @@
 #include "ptrwrap.h"
 #include "redblacktree.h"
 #include "seqset.h"
+#include "tid.h"

src/snmalloc/ds_core/tid.h

@@ -0,0 +1,26 @@
+#pragma once
+
+#include "snmalloc/stl/atomic.h"
+
+namespace snmalloc
+{
+  /**
+   * @brief Get the an id for the current thread.
+   *
+   * @return the thread id, this should never be the default of
+   * ThreadIdentity. Callers can assume it is a non-default value.
+   *
+   * Note, this is only for debug.  We should not assume that this is unique.
+   */
+  inline size_t debug_get_tid() noexcept
+  {
+    static thread_local size_t tid{0};
+    static stl::Atomic<size_t> tid_source{0};
+
+    if (tid == 0)
+    {
+      tid = ++tid_source;
+    }
+    return tid;
+  }
+} // namespace snmalloc

src/snmalloc/pal/pal.h

@@ -177,7 +177,7 @@ namespace snmalloc
   {
     MessageBuilder<BufferSize> msg{std::forward<Args>(args)...};
     MessageBuilder<BufferSize> msg_tid{
-      "{}: {}", DefaultPal::get_tid(), msg.get_message()};
+      "{}: {}", debug_get_tid(), msg.get_message()};
     DefaultPal::message(msg_tid.get_message());
   }
 } // namespace snmalloc

src/snmalloc/pal/pal_concept.h

@@ -68,19 +68,6 @@ namespace snmalloc
                              } noexcept -> ConceptSame<void>;
                          };
 
-  /**
-   * The Pal must provide a thread id for debugging.  It should not return
-   * the default value of ThreadIdentity, as that is used as not an tid in some
-   * places.
-   */
-  template<typename PAL>
-  concept IsPAL_tid =
-    requires() {
-      {
-        PAL::get_tid()
-        } noexcept -> ConceptSame<typename PAL::ThreadIdentity>;
-    };
-
   /**
    * Absent any feature flags, the PAL must support a crude primitive allocator
    */
@@ -137,7 +124,6 @@ namespace snmalloc
     IsPAL_static_sizes<PAL> &&
     IsPAL_error<PAL> &&
     IsPAL_memops<PAL> &&
-    IsPAL_tid<PAL> &&
     (!pal_supports<Entropy, PAL> || IsPAL_get_entropy64<PAL>) &&
     (!pal_supports<LowMemoryNotification, PAL> || IsPAL_mem_low_notify<PAL>) &&
     (pal_supports<NoAllocation, PAL> ||

src/snmalloc/pal/pal_open_enclave.h

@@ -1,7 +1,6 @@
 #pragma once
 
 #include "pal_noalloc.h"
-#include "pal_tid_default.h"
 
 #ifdef OPEN_ENCLAVE
 extern "C" void* oe_memset_s(void* p, size_t p_size, int c, size_t size);
@@ -27,7 +26,7 @@ namespace snmalloc
 
   using OpenEnclaveBasePAL = PALNoAlloc<OpenEnclaveErrorHandler>;
 
-  class PALOpenEnclave : public OpenEnclaveBasePAL, public PalTidDefault
+  class PALOpenEnclave : public OpenEnclaveBasePAL
   {
   public:
     /**

src/snmalloc/pal/pal_posix.h

@@ -1,7 +1,6 @@
 #pragma once
 
 #include "../aal/aal.h"
-#include "pal_tid_default.h"
 #include "pal_timer_default.h"
 #if defined(SNMALLOC_BACKTRACE_HEADER)
 #  include SNMALLOC_BACKTRACE_HEADER
@@ -40,8 +39,7 @@ namespace snmalloc
    * working when an early-malloc error appears.
    */
   template<class OS, auto writev = ::writev, auto fsync = ::fsync>
-  class PALPOSIX : public PalTimerDefaultImpl<PALPOSIX<OS>>,
-                   public PalTidDefault
+  class PALPOSIX : public PalTimerDefaultImpl<PALPOSIX<OS>>
   {
     /**
      * Helper class to access the `default_mmap_flags` field of `OS` if one

src/snmalloc/pal/pal_tid_default.h

@@ -1,30 +0,0 @@
-#pragma once
-
-#include "snmalloc/stl/atomic.h"
-
-namespace snmalloc
-{
-  class PalTidDefault
-  {
-  public:
-    using ThreadIdentity = size_t;
-
-    /**
-     * @brief Get the an id for the current thread.
-     *
-     * @return the thread id, this should never be the default of
-     * ThreadIdentity. Callers can assume it is a non-default value.
-     */
-    static inline ThreadIdentity get_tid() noexcept
-    {
-      static thread_local size_t tid{0};
-      static stl::Atomic<size_t> tid_source{0};
-
-      if (tid == 0)
-      {
-        tid = ++tid_source;
-      }
-      return tid;
-    }
-  };
-} // namespace snmalloc

src/snmalloc/pal/pal_windows.h

@@ -1,7 +1,6 @@
 #pragma once
 
 #include "../aal/aal.h"
-#include "pal_tid_default.h"
 #include "pal_timer_default.h"
 
 #ifdef _WIN32
@@ -26,8 +25,7 @@
 
 namespace snmalloc
 {
-  class PALWindows : public PalTimerDefaultImpl<PALWindows>,
-                     public PalTidDefault
+  class PALWindows : public PalTimerDefaultImpl<PALWindows>
   {
     /**
      * A flag indicating that we have tried to register for low-memory