Merge pull request #69 from kswiecicki/pool-create-owning

Add the umfPoolCreateEx internal function

Author: bratpiorka

src/memory_pool.c

@@ -15,6 +15,37 @@
 #include <assert.h>
 #include <stdlib.h>
 
+umf_result_t umfPoolCreateEx(const umf_memory_pool_ops_t *pool_ops,
+                             void *pool_params,
+                             const umf_memory_provider_ops_t *provider_ops,
+                             void *provider_params,
+                             umf_memory_pool_handle_t *hPool) {
+    if (!pool_ops || !provider_ops || !hPool) {
+        return UMF_RESULT_ERROR_INVALID_ARGUMENT;
+    }
+
+    umf_memory_provider_handle_t provider = NULL;
+    umf_result_t ret =
+        umfMemoryProviderCreate(provider_ops, provider_params, &provider);
+    if (ret != UMF_RESULT_SUCCESS) {
+        return ret;
+    }
+    assert(provider != NULL);
+
+    umf_memory_pool_handle_t pool = NULL;
+    ret = umfPoolCreate(pool_ops, provider, pool_params, &pool);
+    if (ret != UMF_RESULT_SUCCESS) {
+        umfMemoryProviderDestroy(provider);
+        return ret;
+    }
+    assert(pool != NULL);
+
+    pool->own_provider = true;
+    *hPool = pool;
+
+    return UMF_RESULT_SUCCESS;
+}
+
 void *umfPoolMalloc(umf_memory_pool_handle_t hPool, size_t size) {
     return hPool->ops.malloc(hPool->pool_priv, size);
 }

src/memory_pool_default.c

@@ -30,6 +30,7 @@ umf_result_t umfPoolCreate(const umf_memory_pool_ops_t *ops,
     assert(ops->version == UMF_VERSION_CURRENT);
 
     pool->provider = provider;
+    pool->own_provider = false;
 
     pool->ops = *ops;
     ret = ops->initialize(pool->provider, params, &pool->pool_priv);
@@ -44,6 +45,12 @@ umf_result_t umfPoolCreate(const umf_memory_pool_ops_t *ops,
 
 void umfPoolDestroy(umf_memory_pool_handle_t hPool) {
     hPool->ops.finalize(hPool->pool_priv);
+    if (hPool->own_provider) {
+        // Destroy associated memory provider.
+        umf_memory_provider_handle_t hProvider = NULL;
+        umfPoolGetMemoryProvider(hPool, &hProvider);
+        umfMemoryProviderDestroy(hProvider);
+    }
     free(hPool);
 }
 

src/memory_pool_internal.h

@@ -11,9 +11,12 @@
 #define UMF_MEMORY_POOL_INTERNAL_H 1
 
 #include <umf/base.h>
+#include <umf/memory_pool.h>
 #include <umf/memory_pool_ops.h>
 #include <umf/memory_provider.h>
 
+#include <stdbool.h>
+
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -24,8 +27,16 @@ typedef struct umf_memory_pool_t {
 
     // Memory provider used by the pool.
     umf_memory_provider_handle_t provider;
+    // Tells whether memory provider is owned by the pool.
+    bool own_provider;
 } umf_memory_pool_t;
 
+umf_result_t umfPoolCreateEx(const umf_memory_pool_ops_t *pool_ops,
+                             void *pool_params,
+                             const umf_memory_provider_ops_t *provider_ops,
+                             void *provider_params,
+                             umf_memory_pool_handle_t *hPool);
+
 #ifdef __cplusplus
 }
 #endif

src/memory_pool_tracking.c

@@ -36,6 +36,7 @@ umf_result_t umfPoolCreate(const umf_memory_pool_ops_t *ops,
     if (ret != UMF_RESULT_SUCCESS) {
         goto err_provider_create;
     }
+    pool->own_provider = false;
 
     pool->ops = *ops;
     ret = ops->initialize(pool->provider, params, &pool->pool_priv);
@@ -56,6 +57,13 @@ umf_result_t umfPoolCreate(const umf_memory_pool_ops_t *ops,
 
 void umfPoolDestroy(umf_memory_pool_handle_t hPool) {
     hPool->ops.finalize(hPool->pool_priv);
+    if (hPool->own_provider) {
+        // Destroy associated memory provider.
+        umf_memory_provider_handle_t hProvider = NULL;
+        umfPoolGetMemoryProvider(hPool, &hProvider);
+        umfMemoryProviderDestroy(hProvider);
+    }
+    // Destroy tracking provider.
     umfMemoryProviderDestroy(hPool->provider);
     free(hPool);
 }

test/CMakeLists.txt

@@ -66,6 +66,7 @@ add_umf_test(NAME memoryPool SRCS memoryPoolAPI.cpp malloc_compliance_tests.cpp
 add_umf_test(NAME memoryProvider SRCS memoryProviderAPI.cpp)
 add_umf_test(NAME disjointPool SRCS disjoint_pool.cpp malloc_compliance_tests.cpp)
 add_umf_test(NAME c_api_disjoint_pool SRCS c_api/disjoint_pool.c)
+add_umf_test(NAME memory_pool_internal SRCS memory_pool_internal.cpp)
 
 if(LINUX) # OS-specific functions are implemented only for Linux now
     add_umf_test(NAME provider_os_memory SRCS provider_os_memory.cpp LIBS numa)

test/memory_pool_internal.cpp

@@ -0,0 +1,102 @@
+// Copyright (C) 2023 Intel Corporation
+// Under the Apache License v2.0 with LLVM Exceptions. See LICENSE.TXT.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+#include "../src/memory_pool_internal.h"
+#include "base.hpp"
+#include "common/provider_null.h"
+#include "pool.hpp"
+#include "provider_trace.h"
+#include "test_helpers.h"
+
+#include <unordered_map>
+
+using umf_test::test;
+
+template <typename T> umf_memory_pool_ops_t poolNoParamsMakeCOps() {
+    umf_memory_pool_ops_t ops = umf::detail::poolOpsBase<T>();
+
+    ops.initialize = [](umf_memory_provider_handle_t provider, void *params,
+                        void **obj) {
+        (void)params;
+        try {
+            *obj = new T;
+        } catch (...) {
+            return UMF_RESULT_ERROR_OUT_OF_HOST_MEMORY;
+        }
+
+        return umf::detail::initialize<T>(reinterpret_cast<T *>(*obj),
+                                          std::make_tuple(provider));
+    };
+
+    return ops;
+}
+
+umf_memory_pool_ops_t PROXY_POOL_OPS =
+    poolNoParamsMakeCOps<umf_test::proxy_pool>();
+
+TEST_F(test, poolCreateExSuccess) {
+    umf_memory_pool_handle_t pool = nullptr;
+    auto ret = umfPoolCreateEx(&PROXY_POOL_OPS, nullptr, &UMF_NULL_PROVIDER_OPS,
+                               nullptr, &pool);
+    ASSERT_EQ(ret, UMF_RESULT_SUCCESS);
+    ASSERT_NE(pool, nullptr);
+
+    umfPoolDestroy(pool);
+}
+
+TEST_F(test, poolCreateExNullPoolOps) {
+    umf_memory_pool_handle_t pool = nullptr;
+    auto ret = umfPoolCreateEx(nullptr, nullptr, &UMF_NULL_PROVIDER_OPS,
+                               nullptr, &pool);
+    ASSERT_EQ(ret, UMF_RESULT_ERROR_INVALID_ARGUMENT);
+}
+
+TEST_F(test, poolCreateExNullProviderOps) {
+    umf_memory_pool_handle_t pool = nullptr;
+    auto ret =
+        umfPoolCreateEx(&PROXY_POOL_OPS, nullptr, nullptr, nullptr, &pool);
+    ASSERT_EQ(ret, UMF_RESULT_ERROR_INVALID_ARGUMENT);
+}
+
+TEST_F(test, poolCreateExNullPoolHandle) {
+    auto ret = umfPoolCreateEx(&PROXY_POOL_OPS, nullptr, &UMF_NULL_PROVIDER_OPS,
+                               nullptr, nullptr);
+    ASSERT_EQ(ret, UMF_RESULT_ERROR_INVALID_ARGUMENT);
+}
+
+TEST_F(test, poolCreateExCountProviderCalls) {
+    auto nullProvider = umf_test::wrapProviderUnique(nullProviderCreate());
+
+    static std::unordered_map<std::string, size_t> providerCalls;
+    auto traceCb = [](const char *name) { providerCalls[name]++; };
+
+    umf_provider_trace_params_t provider_params = {nullProvider.get(), traceCb};
+
+    umf_memory_pool_handle_t pool = nullptr;
+    umf_result_t ret =
+        umfPoolCreateEx(&PROXY_POOL_OPS, nullptr, &UMF_TRACE_PROVIDER_OPS,
+                        &provider_params, &pool);
+    ASSERT_EQ(ret, UMF_RESULT_SUCCESS);
+    ASSERT_NE(pool, nullptr);
+
+    size_t provider_call_count = 0;
+
+    umfPoolMalloc(pool, 0);
+    ASSERT_EQ(providerCalls["alloc"], 1);
+    ASSERT_EQ(providerCalls.size(), ++provider_call_count);
+
+    umfPoolFree(pool, 0);
+    ASSERT_EQ(providerCalls["free"], 1);
+    ASSERT_EQ(providerCalls.size(), ++provider_call_count);
+
+    umfPoolCalloc(pool, 0, 0);
+    ASSERT_EQ(providerCalls["alloc"], 2);
+    ASSERT_EQ(providerCalls.size(), provider_call_count);
+
+    umfPoolAlignedMalloc(pool, 0, 0);
+    ASSERT_EQ(providerCalls["alloc"], 3);
+    ASSERT_EQ(providerCalls.size(), provider_call_count);
+
+    umfPoolDestroy(pool);
+}