Merge pull request #248 from ldorau/Implement_and_use_umf_ba_linear_free_in_the_proxy_library

Implement and use umf_ba_linear_free() in the proxy library

Author: bratpiorka

src/base_alloc/base_alloc_linear.c

@@ -13,6 +13,18 @@
 #include "utils_common.h"
 #include "utils_concurrency.h"
 
+#ifndef NDEBUG
+#define DEBUG_RUN_CHECKS(pool) ba_debug_checks(pool)
+#define DEBUG_SET_VAR(var, value) DO_WHILE_EXPRS((var = value))
+#define DEBUG_INC_VAR(var) DO_WHILE_EXPRS((var++))
+#define DEBUG_DEC_VAR(var) DO_WHILE_EXPRS((var--))
+#else
+#define DEBUG_RUN_CHECKS(pool) DO_WHILE_EMPTY
+#define DEBUG_SET_VAR(var, value) DO_WHILE_EMPTY
+#define DEBUG_INC_VAR(var) DO_WHILE_EMPTY
+#define DEBUG_DEC_VAR(var) DO_WHILE_EMPTY
+#endif /* NDEBUG */
+
 // minimum size of a single pool of the linear base allocator
 #define MINIMUM_LINEAR_POOL_SIZE (ba_os_get_page_size())
 
@@ -27,9 +39,11 @@ typedef struct umf_ba_main_linear_pool_meta_t {
     os_mutex_t lock;
     char *data_ptr;
     size_t size_left;
+    size_t pool_n_allocs; // number of allocations in this pool
 #ifndef NDEBUG
     size_t n_pools;
-#endif /* NDEBUG */
+    size_t global_n_allocs; // global number of allocations in all pools
+#endif                      /* NDEBUG */
 } umf_ba_main_linear_pool_meta_t;
 
 // the main pool of the linear base allocator (there is only one such pool)
@@ -52,7 +66,8 @@ struct umf_ba_next_linear_pool_t {
     // to be freed in umf_ba_linear_destroy())
     umf_ba_next_linear_pool_t *next_pool;
 
-    size_t pool_size; // size of this pool (argument of ba_os_alloc() call)
+    size_t pool_size;     // size of this pool (argument of ba_os_alloc() call)
+    size_t pool_n_allocs; // number of allocations in this pool
 
     // data area of all pools except of the main (the first one) starts here
     char data[];
@@ -94,9 +109,9 @@ umf_ba_linear_pool_t *umf_ba_linear_create(size_t pool_size) {
     pool->metadata.data_ptr = data_ptr;
     pool->metadata.size_left = size_left;
     pool->next_pool = NULL; // this is the only pool now
-#ifndef NDEBUG
-    pool->metadata.n_pools = 1;
-#endif /* NDEBUG */
+    pool->metadata.pool_n_allocs = 0;
+    DEBUG_SET_VAR(pool->metadata.n_pools, 1);
+    DEBUG_SET_VAR(pool->metadata.global_n_allocs, 0);
 
     // init lock
     os_mutex_t *lock = util_mutex_init(&pool->metadata.lock);
@@ -131,6 +146,7 @@ void *umf_ba_linear_alloc(umf_ba_linear_pool_t *pool, size_t size) {
         }
 
         new_pool->pool_size = pool_size;
+        new_pool->pool_n_allocs = 0;
 
         void *data_ptr = &new_pool->data;
         size_t size_left =
@@ -143,23 +159,86 @@ void *umf_ba_linear_alloc(umf_ba_linear_pool_t *pool, size_t size) {
         // add the new pool to the list of pools
         new_pool->next_pool = pool->next_pool;
         pool->next_pool = new_pool;
-#ifndef NDEBUG
-        pool->metadata.n_pools++;
-#endif /* NDEBUG */
+        DEBUG_INC_VAR(pool->metadata.n_pools);
     }
 
     assert(pool->metadata.size_left >= aligned_size);
     void *ptr = pool->metadata.data_ptr;
     pool->metadata.data_ptr += aligned_size;
     pool->metadata.size_left -= aligned_size;
-#ifndef NDEBUG
-    ba_debug_checks(pool);
-#endif /* NDEBUG */
+    if (pool->next_pool) {
+        pool->next_pool->pool_n_allocs++;
+    } else {
+        pool->metadata.pool_n_allocs++;
+    }
+    DEBUG_INC_VAR(pool->metadata.global_n_allocs);
+    DEBUG_RUN_CHECKS(pool);
     util_mutex_unlock(&pool->metadata.lock);
 
     return ptr;
 }
 
+// check if ptr belongs to pool
+static inline int pool_contains_ptr(void *pool, size_t pool_size,
+                                    void *data_begin, void *ptr) {
+    return ((char *)ptr >= (char *)data_begin &&
+            (char *)ptr < ((char *)(pool)) + pool_size);
+}
+
+// umf_ba_linear_free() really frees memory only if all allocations from an inactive pool were freed
+// It returns:
+// 0  - ptr belonged to the pool and was freed
+// -1 - ptr doesn't belong to the pool and wasn't freed
+int umf_ba_linear_free(umf_ba_linear_pool_t *pool, void *ptr) {
+    util_mutex_lock(&pool->metadata.lock);
+    DEBUG_RUN_CHECKS(pool);
+    if (pool_contains_ptr(pool, pool->metadata.pool_size, pool->data, ptr)) {
+        pool->metadata.pool_n_allocs--;
+        DEBUG_DEC_VAR(pool->metadata.global_n_allocs);
+        size_t page_size = ba_os_get_page_size();
+        if ((pool->metadata.pool_n_allocs == 0) && pool->next_pool &&
+            (pool->metadata.pool_size > page_size)) {
+            // we can free the first (main) pool except of the first page containing the metadata
+            void *ptr = pool + page_size;
+            size_t size = pool->metadata.pool_size - page_size;
+            ba_os_free(ptr, size);
+        }
+        DEBUG_RUN_CHECKS(pool);
+        util_mutex_unlock(&pool->metadata.lock);
+        return 0;
+    }
+
+    umf_ba_next_linear_pool_t *next_pool = pool->next_pool;
+    umf_ba_next_linear_pool_t *prev_pool = NULL;
+    while (next_pool) {
+        if (pool_contains_ptr(next_pool, next_pool->pool_size, next_pool->data,
+                              ptr)) {
+            DEBUG_DEC_VAR(pool->metadata.global_n_allocs);
+            next_pool->pool_n_allocs--;
+            // pool->next_pool is the active pool - we cannot free it
+            if ((next_pool->pool_n_allocs == 0) &&
+                next_pool != pool->next_pool) {
+                assert(prev_pool); // it cannot be the active pool
+                assert(prev_pool->next_pool == next_pool);
+                prev_pool->next_pool = next_pool->next_pool;
+                DEBUG_DEC_VAR(pool->metadata.n_pools);
+                void *ptr = next_pool;
+                size_t size = next_pool->pool_size;
+                ba_os_free(ptr, size);
+            }
+            DEBUG_RUN_CHECKS(pool);
+            util_mutex_unlock(&pool->metadata.lock);
+            return 0;
+        }
+        prev_pool = next_pool;
+        next_pool = next_pool->next_pool;
+    }
+
+    util_mutex_unlock(&pool->metadata.lock);
+    // ptr doesn't belong to the pool and wasn't freed
+    return -1;
+}
+
 void umf_ba_linear_destroy(umf_ba_linear_pool_t *pool) {
     // Do not destroy if we are running in the proxy library,
     // because it may need those resources till
@@ -169,7 +248,12 @@ void umf_ba_linear_destroy(umf_ba_linear_pool_t *pool) {
     }
 
 #ifndef NDEBUG
-    ba_debug_checks(pool);
+    DEBUG_RUN_CHECKS(pool);
+    if (pool->metadata.global_n_allocs) {
+        fprintf(stderr, "umf_ba_linear_destroy(): global_n_allocs = %zu\n",
+                pool->metadata.global_n_allocs);
+        assert(pool->metadata.global_n_allocs == 0);
+    }
 #endif /* NDEBUG */
 
     umf_ba_next_linear_pool_t *current_pool;

src/base_alloc/base_alloc_linear.h

@@ -29,6 +29,12 @@ void umf_ba_linear_destroy(umf_ba_linear_pool_t *pool);
 size_t umf_ba_linear_pool_contains_pointer(umf_ba_linear_pool_t *pool,
                                            void *ptr);
 
+// umf_ba_linear_free() really frees memory only if all allocations from an inactive pool were freed
+// It returns:
+// 0  - ptr belonged to the pool and was freed
+// -1 - ptr doesn't belong to the pool and wasn't freed
+int umf_ba_linear_free(umf_ba_linear_pool_t *pool, void *ptr);
+
 #ifdef __cplusplus
 }
 #endif

src/proxy_lib/proxy_lib.c

@@ -127,6 +127,9 @@ static inline void *ba_generic_realloc(umf_ba_linear_pool_t *pool, void *ptr,
 
     memcpy(new_ptr, ptr, new_size);
 
+    // we can free the old ptr now
+    umf_ba_linear_free(pool, ptr);
+
     return new_ptr;
 }
 
@@ -136,7 +139,7 @@ static inline void *ba_generic_realloc(umf_ba_linear_pool_t *pool, void *ptr,
 
 static void ba_leak_create(void) { Base_alloc_leak = umf_ba_linear_create(0); }
 
-// it does not implement destroy(), because it will not free memory at all
+// it does not implement destroy(), because we cannot destroy non-freed memory
 
 static inline void *ba_leak_malloc(size_t size) {
     util_init_once(&Base_alloc_leak_initialized, ba_leak_create);
@@ -160,6 +163,10 @@ static inline void *ba_leak_aligned_alloc(size_t alignment, size_t size) {
     return (void *)ALIGN_UP((uintptr_t)ptr, alignment);
 }
 
+static inline int ba_leak_free(void *ptr) {
+    return umf_ba_linear_free(Base_alloc_leak, ptr);
+}
+
 static inline size_t ba_leak_pool_contains_pointer(void *ptr) {
     return umf_ba_linear_pool_contains_pointer(Base_alloc_leak, ptr);
 }
@@ -221,8 +228,7 @@ void free(void *ptr) {
         return;
     }
 
-    if (ba_leak_pool_contains_pointer(ptr)) {
-        // allocations from the leak linear base allocator will not be freed at all
+    if (ba_leak_free(ptr) == 0) {
         return;
     }
 

src/utils/utils_common.h

@@ -19,6 +19,14 @@
 extern "C" {
 #endif
 
+#define DO_WHILE_EMPTY                                                         \
+    do {                                                                       \
+    } while (0)
+#define DO_WHILE_EXPRS(expression)                                             \
+    do {                                                                       \
+        expression;                                                            \
+    } while (0)
+
 #ifdef _WIN32 /* Windows */
 
 #define __TLS __declspec(thread)

test/test_base_alloc_linear.cpp

@@ -25,6 +25,8 @@ TEST_F(test, baseAllocLinearAllocMoreThanPoolSize) {
     void *ptr = umf_ba_linear_alloc(pool.get(), new_size);
     UT_ASSERTne(ptr, NULL);
     memset(ptr, 0, new_size);
+
+    umf_ba_linear_free(pool.get(), ptr);
 }
 
 TEST_F(test, baseAllocLinearPoolContainsPointer) {
@@ -43,6 +45,8 @@ TEST_F(test, baseAllocLinearPoolContainsPointer) {
     // assert pool does NOT contain pointer 0x0123
     UT_ASSERTeq(umf_ba_linear_pool_contains_pointer(pool.get(), (void *)0x0123),
                 0);
+
+    umf_ba_linear_free(pool.get(), ptr);
 }
 
 TEST_F(test, baseAllocLinearMultiThreadedAllocMemset) {
@@ -77,6 +81,10 @@ TEST_F(test, baseAllocLinearMultiThreadedAllocMemset) {
                 UT_ASSERTeq(*(buffer[i].ptr + k), (i + TID) & 0xFF);
             }
         }
+
+        for (int i = 0; i < ITERATIONS; i++) {
+            umf_ba_linear_free(pool, buffer[i].ptr);
+        }
     };
 
     std::vector<std::thread> threads;