Merge pull request #885 from vinser52/svinogra_scalable_config

Introduce config params for scalable pool

Author: bratpiorka

include/umf/pools/pool_scalable.h

@@ -14,9 +14,47 @@
 extern "C" {
 #endif
 
+#include <stdbool.h>
+
 #include <umf/memory_pool.h>
 #include <umf/memory_provider.h>
 
+struct umf_scalable_pool_params_t;
+
+/// @brief handle to the parameters of the scalable pool.
+typedef struct umf_scalable_pool_params_t *umf_scalable_pool_params_handle_t;
+
+/// @brief  Create a struct to store parameters of scalable pool.
+/// @param  hParams [out] handle to the newly created parameters struct.
+/// @return UMF_RESULT_SUCCESS on success or appropriate error code on failure.
+umf_result_t
+umfScalablePoolParamsCreate(umf_scalable_pool_params_handle_t *hParams);
+
+/// @brief  Destroy parameters struct.
+/// @param  hParams handle to the parameters of the scalable pool.
+/// @return UMF_RESULT_SUCCESS on success or appropriate error code on failure.
+umf_result_t
+umfScalablePoolParamsDestroy(umf_scalable_pool_params_handle_t hParams);
+
+/// @brief  Set granularity of allocations that scalable pool requests from a memory provider.
+/// @param  hParams handle to the parameters of the scalable pool.
+/// @param  granularity granularity in bytes.
+/// @return UMF_RESULT_SUCCESS on success or appropriate error code on failure.
+umf_result_t
+umfScalablePoolParamsSetGranularity(umf_scalable_pool_params_handle_t hParams,
+                                    size_t granularity);
+
+/// @brief  Set if scalable pool should keep all memory allocated from memory provider till destruction.
+/// @param  hParams handle to the parameters of the scalable pool.
+/// @param  keepAllMemory \p true if the scalable pool should not call
+/// \p umfMemoryProviderFree until it is destroyed, \p false otherwise.
+/// @return UMF_RESULT_SUCCESS on success or appropriate error code on failure.
+umf_result_t
+umfScalablePoolParamsSetKeepAllMemory(umf_scalable_pool_params_handle_t hParams,
+                                      bool keepAllMemory);
+
+/// @brief  Return \p ops structure containing pointers to the scalable pool implementation.
+/// @return pointer to the \p umf_memory_pool_ops_t struct.
 umf_memory_pool_ops_t *umfScalablePoolOps(void);
 
 #ifdef __cplusplus

src/cpp_helpers.hpp

@@ -79,7 +79,7 @@ template <typename T> umf_memory_pool_ops_t poolOpsBase() {
     return ops;
 }
 
-template <typename T> umf_memory_provider_ops_t providerOpsBase() {
+template <typename T> constexpr umf_memory_provider_ops_t providerOpsBase() {
     umf_memory_provider_ops_t ops{};
     ops.version = UMF_VERSION_CURRENT;
     ops.finalize = [](void *obj) { delete reinterpret_cast<T *>(obj); };
@@ -134,7 +134,7 @@ template <typename T, typename ParamType> umf_memory_pool_ops_t poolMakeCOps() {
 // C API. 'params' from ops.initialize will be casted to 'ParamType*'
 // and passed to T::initialize() function.
 template <typename T, typename ParamType>
-umf_memory_provider_ops_t providerMakeCOps() {
+constexpr umf_memory_provider_ops_t providerMakeCOps() {
     umf_memory_provider_ops_t ops = detail::providerOpsBase<T>();
 
     ops.initialize = []([[maybe_unused]] void *params, void **obj) {

src/libumf.def

@@ -82,3 +82,7 @@ EXPORTS
     umfProxyPoolOps
     umfPutIPCHandle
     umfScalablePoolOps
+    umfScalablePoolParamsCreate
+    umfScalablePoolParamsDestroy
+    umfScalablePoolParamsSetGranularity
+    umfScalablePoolParamsSetKeepAllMemory

src/libumf.map

@@ -76,6 +76,10 @@ UMF_1.0 {
         umfProxyPoolOps;
         umfPutIPCHandle;
         umfScalablePoolOps;
+        umfScalablePoolParamsCreate;
+        umfScalablePoolParamsDestroy;
+        umfScalablePoolParamsSetGranularity;
+        umfScalablePoolParamsSetKeepAllMemory;
     local:
         *;
 };

src/pool/pool_scalable.c

@@ -31,6 +31,7 @@ typedef void (*raw_free_tbb_type)(intptr_t, void *, size_t);
 static __TLS umf_result_t TLS_last_allocation_error;
 static __TLS umf_result_t TLS_last_free_error;
 
+static const size_t DEFAULT_GRANULARITY = 2 * 1024 * 1024; // 2MB
 typedef struct tbb_mem_pool_policy_t {
     raw_alloc_tbb_type pAlloc;
     raw_free_tbb_type pFree;
@@ -39,6 +40,11 @@ typedef struct tbb_mem_pool_policy_t {
     unsigned fixed_pool : 1, keep_all_memory : 1, reserved : 30;
 } tbb_mem_pool_policy_t;
 
+typedef struct umf_scalable_pool_params_t {
+    size_t granularity;
+    bool keep_all_memory;
+} umf_scalable_pool_params_t;
+
 typedef struct tbb_callbacks_t {
     void *(*pool_malloc)(void *, size_t);
     void *(*pool_realloc)(void *, void *, size_t);
@@ -167,19 +173,88 @@ static void tbb_raw_free_wrapper(intptr_t pool_id, void *ptr, size_t bytes) {
     }
 }
 
+umf_result_t
+umfScalablePoolParamsCreate(umf_scalable_pool_params_handle_t *params) {
+    if (!params) {
+        LOG_ERR("scalable pool params handle is NULL");
+        return UMF_RESULT_ERROR_INVALID_ARGUMENT;
+    }
+
+    umf_scalable_pool_params_t *params_data =
+        umf_ba_global_alloc(sizeof(umf_scalable_pool_params_t));
+    if (!params_data) {
+        LOG_ERR("cannot allocate memory for scalable poolparams");
+        return UMF_RESULT_ERROR_OUT_OF_HOST_MEMORY;
+    }
+
+    params_data->granularity = DEFAULT_GRANULARITY;
+    params_data->keep_all_memory = false;
+
+    *params = (umf_scalable_pool_params_handle_t)params_data;
+
+    return UMF_RESULT_SUCCESS;
+}
+
+umf_result_t
+umfScalablePoolParamsDestroy(umf_scalable_pool_params_handle_t params) {
+    if (!params) {
+        LOG_ERR("params is NULL");
+        return UMF_RESULT_ERROR_INVALID_ARGUMENT;
+    }
+
+    umf_ba_global_free(params);
+
+    return UMF_RESULT_SUCCESS;
+}
+
+umf_result_t
+umfScalablePoolParamsSetGranularity(umf_scalable_pool_params_handle_t params,
+                                    size_t granularity) {
+    if (!params) {
+        LOG_ERR("params is NULL");
+        return UMF_RESULT_ERROR_INVALID_ARGUMENT;
+    }
+
+    if (granularity == 0) {
+        LOG_ERR("granularity cannot be 0");
+        return UMF_RESULT_ERROR_INVALID_ARGUMENT;
+    }
+
+    params->granularity = granularity;
+
+    return UMF_RESULT_SUCCESS;
+}
+
+umf_result_t
+umfScalablePoolParamsSetKeepAllMemory(umf_scalable_pool_params_handle_t params,
+                                      bool keep_all_memory) {
+    if (!params) {
+        LOG_ERR("params is NULL");
+        return UMF_RESULT_ERROR_INVALID_ARGUMENT;
+    }
+
+    params->keep_all_memory = keep_all_memory;
+
+    return UMF_RESULT_SUCCESS;
+}
+
 static umf_result_t tbb_pool_initialize(umf_memory_provider_handle_t provider,
                                         void *params, void **pool) {
-    (void)params; // unused
-
-    const size_t GRANULARITY = 2 * 1024 * 1024;
     tbb_mem_pool_policy_t policy = {.pAlloc = tbb_raw_alloc_wrapper,
                                     .pFree = tbb_raw_free_wrapper,
-                                    .granularity = GRANULARITY,
+                                    .granularity = DEFAULT_GRANULARITY,
                                     .version = 1,
                                     .fixed_pool = false,
                                     .keep_all_memory = false,
                                     .reserved = 0};
 
+    if (params) {
+        umf_scalable_pool_params_handle_t scalable_params =
+            (umf_scalable_pool_params_handle_t)params;
+        policy.granularity = scalable_params->granularity;
+        policy.keep_all_memory = scalable_params->keep_all_memory;
+    }
+
     tbb_memory_pool_t *pool_data =
         umf_ba_global_alloc(sizeof(tbb_memory_pool_t));
     if (!pool_data) {

test/pools/scalable_pool.cpp

@@ -7,10 +7,148 @@
 
 #include "pool.hpp"
 #include "poolFixtures.hpp"
+#include "provider.hpp"
 
 auto defaultParams = umfOsMemoryProviderParamsDefault();
 INSTANTIATE_TEST_SUITE_P(scalablePoolTest, umfPoolTest,
                          ::testing::Values(poolCreateExtParams{
                              umfScalablePoolOps(), nullptr,
                              umfOsMemoryProviderOps(), &defaultParams,
                              nullptr}));
+
+using scalablePoolParams = std::tuple<size_t, bool>;
+struct umfScalablePoolParamsTest
+    : umf_test::test,
+      ::testing::WithParamInterface<scalablePoolParams> {
+
+    struct validation_params_t {
+        size_t granularity;
+        bool keep_all_memory;
+    };
+
+    struct provider_validator : public umf_test::provider_ba_global {
+        using base_provider = umf_test::provider_ba_global;
+
+        umf_result_t initialize(validation_params_t *params) noexcept {
+            EXPECT_NE(params, nullptr);
+            expected_params = params;
+            return UMF_RESULT_SUCCESS;
+        }
+        umf_result_t alloc(size_t size, size_t align, void **ptr) noexcept {
+            EXPECT_EQ(size, expected_params->granularity);
+            return base_provider::alloc(size, align, ptr);
+        }
+        umf_result_t free(void *ptr, size_t size) noexcept {
+            EXPECT_EQ(expected_params->keep_all_memory, false);
+            return base_provider::free(ptr, size);
+        }
+
+        validation_params_t *expected_params;
+    };
+
+    static constexpr umf_memory_provider_ops_t VALIDATOR_PROVIDER_OPS =
+        umf::providerMakeCOps<provider_validator, validation_params_t>();
+
+    umfScalablePoolParamsTest() {}
+    void SetUp() override {
+        test::SetUp();
+        auto [granularity, keep_all_memory] = this->GetParam();
+        expected_params.granularity = granularity;
+        expected_params.keep_all_memory = keep_all_memory;
+        umf_result_t ret = umfScalablePoolParamsCreate(&params);
+        ASSERT_EQ(ret, UMF_RESULT_SUCCESS);
+        ret = umfScalablePoolParamsSetGranularity(params, granularity);
+        ASSERT_EQ(ret, UMF_RESULT_SUCCESS);
+        ret = umfScalablePoolParamsSetKeepAllMemory(params, keep_all_memory);
+        ASSERT_EQ(ret, UMF_RESULT_SUCCESS);
+    }
+
+    void TearDown() override {
+        umfScalablePoolParamsDestroy(params);
+        test::TearDown();
+    }
+
+    umf::pool_unique_handle_t makePool() {
+        umf_memory_provider_handle_t hProvider = nullptr;
+        umf_memory_pool_handle_t hPool = nullptr;
+
+        auto ret = umfMemoryProviderCreate(&VALIDATOR_PROVIDER_OPS,
+                                           &expected_params, &hProvider);
+        EXPECT_EQ(ret, UMF_RESULT_SUCCESS);
+
+        ret = umfPoolCreate(umfScalablePoolOps(), hProvider, params,
+                            UMF_POOL_CREATE_FLAG_OWN_PROVIDER, &hPool);
+        EXPECT_EQ(ret, UMF_RESULT_SUCCESS);
+
+        return umf::pool_unique_handle_t(hPool, &umfPoolDestroy);
+    }
+
+    void allocFreeFlow() {
+        static const size_t ALLOC_SIZE = 128;
+        static const size_t NUM_ALLOCATIONS =
+            expected_params.granularity / ALLOC_SIZE * 20;
+        std::vector<void *> ptrs;
+
+        auto pool = makePool();
+        ASSERT_NE(pool, nullptr);
+
+        for (size_t i = 0; i < NUM_ALLOCATIONS; ++i) {
+            auto *ptr = umfPoolMalloc(pool.get(), ALLOC_SIZE);
+            ASSERT_NE(ptr, nullptr);
+            ptrs.push_back(ptr);
+        }
+
+        for (size_t i = 0; i < NUM_ALLOCATIONS; ++i) {
+            auto ret = umfPoolFree(pool.get(), ptrs[i]);
+            ASSERT_EQ(ret, UMF_RESULT_SUCCESS);
+        }
+
+        // Now pool can call free during pool destruction
+        expected_params.keep_all_memory = false;
+    }
+
+    validation_params_t expected_params;
+    umf_scalable_pool_params_handle_t params;
+};
+
+TEST_P(umfScalablePoolParamsTest, allocFree) { allocFreeFlow(); }
+
+TEST_P(umfScalablePoolParamsTest, updateParams) {
+    expected_params.granularity *= 2;
+    umf_result_t ret = umfScalablePoolParamsSetGranularity(
+        params, expected_params.granularity);
+    ASSERT_EQ(ret, UMF_RESULT_SUCCESS);
+
+    expected_params.keep_all_memory = !expected_params.keep_all_memory;
+    ret = umfScalablePoolParamsSetKeepAllMemory(
+        params, expected_params.keep_all_memory);
+    ASSERT_EQ(ret, UMF_RESULT_SUCCESS);
+
+    allocFreeFlow();
+}
+
+TEST_P(umfScalablePoolParamsTest, invalidParams) {
+    umf_result_t ret = umfScalablePoolParamsCreate(nullptr);
+    ASSERT_EQ(ret, UMF_RESULT_ERROR_INVALID_ARGUMENT);
+
+    ret = umfScalablePoolParamsSetGranularity(nullptr, 2 * 1024 * 1024);
+    ASSERT_EQ(ret, UMF_RESULT_ERROR_INVALID_ARGUMENT);
+
+    ret = umfScalablePoolParamsSetGranularity(params, 0);
+    ASSERT_EQ(ret, UMF_RESULT_ERROR_INVALID_ARGUMENT);
+
+    ret = umfScalablePoolParamsSetKeepAllMemory(nullptr, true);
+    ASSERT_EQ(ret, UMF_RESULT_ERROR_INVALID_ARGUMENT);
+
+    ret = umfScalablePoolParamsSetKeepAllMemory(nullptr, false);
+    ASSERT_EQ(ret, UMF_RESULT_ERROR_INVALID_ARGUMENT);
+
+    ret = umfScalablePoolParamsDestroy(nullptr);
+    ASSERT_EQ(ret, UMF_RESULT_ERROR_INVALID_ARGUMENT);
+}
+
+INSTANTIATE_TEST_SUITE_P(
+    scalablePoolTest, umfScalablePoolParamsTest,
+    testing::Combine(testing::Values(2 * 1024 * 1024, 3 * 1024 * 1024,
+                                     4 * 1024 * 1024, 5 * 1024 * 1024),
+                     testing::Values(false, true)));

test/supp/memcheck-umf_test-scalable_pool.supp

@@ -0,0 +1,18 @@
+{
+   Conditional jump or move depends on uninitialised value(s) - internal issue of libtbbmalloc.so
+   Memcheck:Cond
+   fun:_ZN3rml9pool_freeEPNS_10MemoryPoolEPv
+   fun:tbb_free
+   fun:umfPoolFree
+   ...
+}
+
+{
+   Conditional jump or move depends on uninitialised value(s) - internal issue of libtbbmalloc.so
+   Memcheck:Cond
+   obj:*libtbbmalloc.so*
+   fun:_ZN3rml9pool_freeEPNS_10MemoryPoolEPv
+   fun:tbb_free
+   fun:umfPoolFree
+   ...
+}