[umf] add windows support to critnib

Implement locks using C++ std::mutex and atomics
by using Interlocked* functions and calling _ReadWriteBarrier

Author: igchor

source/common/unified_malloc_framework/CMakeLists.txt

@@ -11,8 +11,10 @@ set(UMF_SOURCES
     src/critnib/critnib.c
 )
 
-if (MSVC)
-    set(UMF_SOURCES ${UMF_SOURCES} src/memory_tracker_windows.cpp)
+if(MSVC)
+    set(UMF_SOURCES ${UMF_SOURCES} src/utils/utils_windows.cpp src/memory_tracker_windows.cpp)
+else()
+    set(UMF_SOURCES ${UMF_SOURCES} src/utils/utils_posix.c)
 endif()
 
 if(UMF_BUILD_SHARED_LIBRARY)

source/common/unified_malloc_framework/src/critnib/critnib.c

@@ -58,8 +58,8 @@
 #include <stdbool.h>
 #include <stddef.h>
 
+#include "../utils/utils.h"
 #include "critnib.h"
-#include "pmdk-compat.h"
 
 /*
  * A node that has been deleted is left untouched for this many delete
@@ -126,25 +126,25 @@ struct critnib {
 
     uint64_t remove_count;
 
-    os_mutex_t mutex; /* writes/removes */
+    struct os_mutex_t *mutex; /* writes/removes */
 };
 
 /*
  * atomic load
  */
 static void load(void *src, void *dst) {
-    __atomic_load((word *)src, (word *)dst, memory_order_acquire);
+    util_atomic_load_acquire((word *)src, (word *)dst);
 }
 
 static void load64(uint64_t *src, uint64_t *dst) {
-    __atomic_load(src, dst, memory_order_acquire);
+    util_atomic_load_acquire(src, dst);
 }
 
 /*
  * atomic store
  */
 static void store(void *dst, void *src) {
-    __atomic_store_n((word *)dst, (word)src, memory_order_release);
+    util_atomic_store_release((word *)dst, (word)src);
 }
 
 /*
@@ -181,7 +181,11 @@ struct critnib *critnib_new(void) {
         return NULL;
     }
 
-    util_mutex_init(&c->mutex);
+    c->mutex = util_mutex_create();
+    if (!c->mutex) {
+        free(c);
+        return NULL;
+    }
 
     VALGRIND_HG_DRD_DISABLE_CHECKING(&c->root, sizeof(c->root));
     VALGRIND_HG_DRD_DISABLE_CHECKING(&c->remove_count, sizeof(c->remove_count));
@@ -214,7 +218,7 @@ void critnib_delete(struct critnib *c) {
         delete_node(c->root);
     }
 
-    util_mutex_destroy(&c->mutex);
+    util_mutex_destroy(c->mutex);
 
     for (struct critnib_node *m = c->deleted_node; m;) {
         struct critnib_node *mm = m->child[0];
@@ -313,11 +317,11 @@ static struct critnib_leaf *alloc_leaf(struct critnib *__restrict c) {
  * Takes a global write lock but doesn't stall any readers.
  */
 int critnib_insert(struct critnib *c, word key, void *value, int update) {
-    util_mutex_lock(&c->mutex);
+    util_mutex_lock(c->mutex);
 
     struct critnib_leaf *k = alloc_leaf(c);
     if (!k) {
-        util_mutex_unlock(&c->mutex);
+        util_mutex_unlock(c->mutex);
 
         return ENOMEM;
     }
@@ -333,7 +337,7 @@ int critnib_insert(struct critnib *c, word key, void *value, int update) {
     if (!n) {
         c->root = kn;
 
-        util_mutex_unlock(&c->mutex);
+        util_mutex_unlock(c->mutex);
 
         return 0;
     }
@@ -351,7 +355,7 @@ int critnib_insert(struct critnib *c, word key, void *value, int update) {
         n = prev;
         store(&n->child[slice_index(key, n->shift)], kn);
 
-        util_mutex_unlock(&c->mutex);
+        util_mutex_unlock(c->mutex);
 
         return 0;
     }
@@ -365,10 +369,10 @@ int critnib_insert(struct critnib *c, word key, void *value, int update) {
 
         if (update) {
             to_leaf(n)->value = value;
-            util_mutex_unlock(&c->mutex);
+            util_mutex_unlock(c->mutex);
             return 0;
         } else {
-            util_mutex_unlock(&c->mutex);
+            util_mutex_unlock(c->mutex);
             return EEXIST;
         }
     }
@@ -380,7 +384,7 @@ int critnib_insert(struct critnib *c, word key, void *value, int update) {
     if (!m) {
         free_leaf(c, to_leaf(kn));
 
-        util_mutex_unlock(&c->mutex);
+        util_mutex_unlock(c->mutex);
 
         return ENOMEM;
     }
@@ -396,7 +400,7 @@ int critnib_insert(struct critnib *c, word key, void *value, int update) {
     m->path = key & path_mask(sh);
     store(parent, m);
 
-    util_mutex_unlock(&c->mutex);
+    util_mutex_unlock(c->mutex);
 
     return 0;
 }
@@ -408,15 +412,14 @@ void *critnib_remove(struct critnib *c, word key) {
     struct critnib_leaf *k;
     void *value = NULL;
 
-    util_mutex_lock(&c->mutex);
+    util_mutex_lock(c->mutex);
 
     struct critnib_node *n = c->root;
     if (!n) {
         goto not_found;
     }
 
-    word del = __atomic_fetch_add(&c->remove_count, 1, __ATOMIC_ACQ_REL) %
-               DELETED_LIFE;
+    word del = (util_atomic_increment(&c->remove_count) - 1) % DELETED_LIFE;
     free_node(c, c->pending_del_nodes[del]);
     free_leaf(c, c->pending_del_leaves[del]);
     c->pending_del_nodes[del] = NULL;
@@ -479,7 +482,7 @@ void *critnib_remove(struct critnib *c, word key) {
     c->pending_del_leaves[del] = k;
 
 not_found:
-    util_mutex_unlock(&c->mutex);
+    util_mutex_unlock(c->mutex);
     return value;
 }
 
@@ -802,9 +805,9 @@ static int iter(struct critnib_node *__restrict n, word min, word max,
 void critnib_iter(critnib *c, uintptr_t min, uintptr_t max,
                   int (*func)(uintptr_t key, void *value, void *privdata),
                   void *privdata) {
-    util_mutex_lock(&c->mutex);
+    util_mutex_lock(c->mutex);
     if (c->root) {
         iter(c->root, min, max, func, privdata);
     }
-    util_mutex_unlock(&c->mutex);
+    util_mutex_unlock(c->mutex);
 }

source/common/unified_malloc_framework/src/memory_tracker_windows.cpp

@@ -8,6 +8,9 @@
  *
  */
 
+#include "critnib/critnib.h"
+#include "memory_tracker.h"
+
 #include <windows.h>
 
 #if defined(UMF_SHARED_LIBRARY)
@@ -23,7 +26,7 @@ BOOL APIENTRY DllMain(HINSTANCE hinstDLL, DWORD fdwReason, LPVOID lpvReserved) {
 #else
 struct tracker_t {
     tracker_t() { map = critnib_new(); }
-    ~tracker_t() { critnib_remove(map); }
+    ~tracker_t() { critnib_delete(map); }
     critnib *map;
 };
 tracker_t TRACKER_INSTANCE;

source/common/unified_malloc_framework/src/critnib/pmdk-compat.h renamed to source/common/unified_malloc_framework/src/utils/utils.h

@@ -8,40 +8,70 @@
  *
  */
 
-#include <pthread.h>
-#include <stdatomic.h>
+#include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#if defined(_WIN32)
+#include <windows.h>
+#else
+#include <stdatomic.h>
+#endif
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+struct os_mutex_t;
+
+struct os_mutex_t *util_mutex_create(void);
+void util_mutex_destroy(struct os_mutex_t *mutex);
+int util_mutex_lock(struct os_mutex_t *mutex);
+int util_mutex_unlock(struct os_mutex_t *mutex);
+
+#if defined(_WIN32)
+static __inline unsigned char util_lssb_index(long long value) {
+    unsigned long ret;
+    _BitScanForward64(&ret, value);
+    return (unsigned char)ret;
+}
+static __inline unsigned char util_mssb_index(long long value) {
+    unsigned long ret;
+    _BitScanReverse64(&ret, value);
+    return (unsigned char)ret;
+}
 
-typedef pthread_mutex_t os_mutex_t;
+// There is no good way to do atomic_load on windows...
+#define util_atomic_load_acquire(object, dest)                                 \
+    do {                                                                       \
+        *dest = InterlockedOr64Acquire((LONG64 volatile *)dest, 0);            \
+    } while (0)
+
+#define util_atomic_store_release(object, desired)                             \
+    InterlockedExchange64((LONG64 volatile *)object, (LONG64)desired)
+#define util_atomic_increment(object)                                          \
+    InterlockedIncrement64((LONG64 volatile *)object)
+#else
+#define util_lssb_index(x) ((unsigned char)__builtin_ctzll(x))
+#define util_mssb_index(x) ((unsigned char)(63 - __builtin_clzll(x)))
+#define util_atomic_load_acquire(object, dest)                                 \
+    __atomic_load(object, dest, memory_order_acquire)
+#define util_atomic_store_release(object, desired)                             \
+    __atomic_store_n(object, desired, memory_order_release)
+#define util_atomic_increment(object)                                          \
+    __atomic_add_fetch(object, 1, __ATOMIC_ACQ_REL)
+#endif
 
 #define Malloc malloc
 #define Free free
 
-static void *Zalloc(size_t s) {
+static inline void *Zalloc(size_t s) {
     void *m = Malloc(s);
     if (m) {
         memset(m, 0, s);
     }
     return m;
 }
 
-#define util_mutex_init(x) pthread_mutex_init(x, NULL)
-#define util_mutex_destroy(x) pthread_mutex_destroy(x)
-#define util_mutex_lock(x) pthread_mutex_lock(x)
-#define util_mutex_unlock(x) pthread_mutex_unlock(x)
-#define util_lssb_index64(x) ((unsigned char)__builtin_ctzll(x))
-#define util_mssb_index64(x) ((unsigned char)(63 - __builtin_clzll(x)))
-#define util_lssb_index32(x) ((unsigned char)__builtin_ctzl(x))
-#define util_mssb_index32(x) ((unsigned char)(31 - __builtin_clzl(x)))
-#if __SIZEOF_LONG__ == 8
-#define util_lssb_index(x) util_lssb_index64(x)
-#define util_mssb_index(x) util_mssb_index64(x)
-#else
-#define util_lssb_index(x) util_lssb_index32(x)
-#define util_mssb_index(x) util_mssb_index32(x)
-#endif
-
 #define NOFUNCTION                                                             \
     do                                                                         \
         ;                                                                      \
@@ -53,7 +83,6 @@ static void *Zalloc(size_t s) {
 #define ASSERT(x) NOFUNCTION
 #define ASSERTne(x, y) ASSERT(x != y)
 #else
-#include <stdio.h>
 #define ASSERT(x)                                                              \
     do                                                                         \
         if (!(x)) {                                                            \
@@ -76,3 +105,7 @@ static void *Zalloc(size_t s) {
         }                                                                      \
     } while (0)
 #endif
+
+#ifdef __cplusplus
+}
+#endif

source/common/unified_malloc_framework/src/utils/utils_posix.c

@@ -0,0 +1,35 @@
+/*
+ *
+ * Copyright (C) 2023 Intel Corporation
+ *
+ * Part of the Unified-Runtime Project, under the Apache License v2.0 with LLVM Exceptions.
+ * See LICENSE.TXT
+ * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+ *
+ */
+
+#include <pthread.h>
+#include <stdlib.h>
+
+#include "utils.h"
+
+struct os_mutex_t *util_mutex_create() {
+    pthread_mutex_t *mutex = (pthread_mutex_t *)malloc(sizeof(pthread_mutex_t));
+    int ret = pthread_mutex_init(mutex, NULL);
+    return ret == 0 ? ((struct os_mutex_t *)mutex) : NULL;
+}
+
+void util_mutex_destroy(struct os_mutex_t *m) {
+    pthread_mutex_t *mutex = (pthread_mutex_t *)m;
+    int ret = pthread_mutex_destroy(mutex);
+    (void)ret; // TODO: add logging
+    free(m);
+}
+
+int util_mutex_lock(struct os_mutex_t *m) {
+    return pthread_mutex_lock((pthread_mutex_t *)m);
+}
+
+int util_mutex_unlock(struct os_mutex_t *m) {
+    return pthread_mutex_unlock((pthread_mutex_t *)m);
+}

source/common/unified_malloc_framework/src/utils/utils_windows.cpp

@@ -0,0 +1,33 @@
+/*
+ *
+ * Copyright (C) 2023 Intel Corporation
+ *
+ * Part of the Unified-Runtime Project, under the Apache License v2.0 with LLVM Exceptions.
+ * See LICENSE.TXT
+ * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+ *
+ */
+
+#include <mutex>
+
+#include "utils.h"
+
+struct os_mutex_t *util_mutex_create(void) {
+    return reinterpret_cast<struct os_mutex_t *>(new std::mutex);
+}
+
+void util_mutex_destroy(struct os_mutex_t *mutex) {
+    delete reinterpret_cast<std::mutex *>(mutex);
+}
+
+int util_mutex_lock(struct os_mutex_t *mutex) try {
+    reinterpret_cast<std::mutex *>(mutex)->lock();
+    return 0;
+} catch (std::system_error &err) {
+    return err.code().value();
+}
+
+int util_mutex_unlock(struct os_mutex_t *mutex) {
+    reinterpret_cast<std::mutex *>(mutex)->unlock();
+    return 0;
+}