From cb120ddc8020cac313a9e03c2314ddf1efba760f Mon Sep 17 00:00:00 2001 From: "Vinogradov, Sergei" Date: Thu, 7 Nov 2024 15:51:02 +0100 Subject: [PATCH 1/2] Introduce config params for scalable pool --- include/umf/pools/pool_scalable.h | 38 ++++++++++++++ src/libumf.def | 4 ++ src/libumf.map | 4 ++ src/pool/pool_scalable.c | 83 +++++++++++++++++++++++++++++-- 4 files changed, 125 insertions(+), 4 deletions(-) diff --git a/include/umf/pools/pool_scalable.h b/include/umf/pools/pool_scalable.h index 3b9945f0b6..072169b68c 100644 --- a/include/umf/pools/pool_scalable.h +++ b/include/umf/pools/pool_scalable.h @@ -14,9 +14,47 @@ extern "C" { #endif +#include + #include #include +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 diff --git a/src/libumf.def b/src/libumf.def index 979187d96f..56e26050c8 100644 --- a/src/libumf.def +++ b/src/libumf.def @@ -82,3 +82,7 @@ EXPORTS umfProxyPoolOps umfPutIPCHandle umfScalablePoolOps + umfScalablePoolParamsCreate + umfScalablePoolParamsDestroy + umfScalablePoolParamsSetGranularity + umfScalablePoolParamsSetKeepAllMemory diff --git a/src/libumf.map b/src/libumf.map index bfca09a270..19235705c9 100644 --- a/src/libumf.map +++ b/src/libumf.map @@ -76,6 +76,10 @@ UMF_1.0 { umfProxyPoolOps; umfPutIPCHandle; umfScalablePoolOps; + umfScalablePoolParamsCreate; + umfScalablePoolParamsDestroy; + umfScalablePoolParamsSetGranularity; + umfScalablePoolParamsSetKeepAllMemory; local: *; }; diff --git a/src/pool/pool_scalable.c b/src/pool/pool_scalable.c index ebf42493c9..26ab7ad637 100644 --- a/src/pool/pool_scalable.c +++ b/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) { From e0ef5d269e0e3af898bc2b4ca89de180ca796f9e Mon Sep 17 00:00:00 2001 From: "Vinogradov, Sergei" Date: Thu, 7 Nov 2024 16:51:58 +0100 Subject: [PATCH 2/2] Add tests for scalable pool params --- src/cpp_helpers.hpp | 4 +- test/pools/scalable_pool.cpp | 138 ++++++++++++++++++ .../supp/memcheck-umf_test-scalable_pool.supp | 18 +++ 3 files changed, 158 insertions(+), 2 deletions(-) create mode 100644 test/supp/memcheck-umf_test-scalable_pool.supp diff --git a/src/cpp_helpers.hpp b/src/cpp_helpers.hpp index aafc7c4db4..6316ccbc7e 100644 --- a/src/cpp_helpers.hpp +++ b/src/cpp_helpers.hpp @@ -79,7 +79,7 @@ template umf_memory_pool_ops_t poolOpsBase() { return ops; } -template umf_memory_provider_ops_t providerOpsBase() { +template 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(obj); }; @@ -134,7 +134,7 @@ template umf_memory_pool_ops_t poolMakeCOps() { // C API. 'params' from ops.initialize will be casted to 'ParamType*' // and passed to T::initialize() function. template -umf_memory_provider_ops_t providerMakeCOps() { +constexpr umf_memory_provider_ops_t providerMakeCOps() { umf_memory_provider_ops_t ops = detail::providerOpsBase(); ops.initialize = []([[maybe_unused]] void *params, void **obj) { diff --git a/test/pools/scalable_pool.cpp b/test/pools/scalable_pool.cpp index 2be29eb9d0..68409b4bba 100644 --- a/test/pools/scalable_pool.cpp +++ b/test/pools/scalable_pool.cpp @@ -7,6 +7,7 @@ #include "pool.hpp" #include "poolFixtures.hpp" +#include "provider.hpp" auto defaultParams = umfOsMemoryProviderParamsDefault(); INSTANTIATE_TEST_SUITE_P(scalablePoolTest, umfPoolTest, @@ -14,3 +15,140 @@ INSTANTIATE_TEST_SUITE_P(scalablePoolTest, umfPoolTest, umfScalablePoolOps(), nullptr, umfOsMemoryProviderOps(), &defaultParams, nullptr})); + +using scalablePoolParams = std::tuple; +struct umfScalablePoolParamsTest + : umf_test::test, + ::testing::WithParamInterface { + + 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(); + + 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(¶ms); + 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 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))); diff --git a/test/supp/memcheck-umf_test-scalable_pool.supp b/test/supp/memcheck-umf_test-scalable_pool.supp new file mode 100644 index 0000000000..114dfb236a --- /dev/null +++ b/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 + ... +}