From f09ed0370262cfb883fa432d8991feb3092b7d40 Mon Sep 17 00:00:00 2001 From: Krzysztof Swiecicki Date: Mon, 24 Jul 2023 14:41:20 +0200 Subject: [PATCH 1/2] [umf] Make pool tracking optional This adds a new CMake option 'UMF_ENABLE_POOL_TRACKING' that enables pool tracking in UMF. Pool tracking is turned off by default. The pool tracking tests were wrapped around the compile definition 'UMF_ENABLE_POOL_TRACKING_TESTS'. --- CMakeLists.txt | 1 + .../unified_malloc_framework/CMakeLists.txt | 6 + .../src/memory_pool.c | 118 +---------------- .../src/memory_pool_default.c | 93 +++++++++++++ .../src/memory_pool_internal.h | 37 ++++++ .../src/memory_pool_tracking.c | 122 ++++++++++++++++++ test/unified_malloc_framework/CMakeLists.txt | 3 + test/unified_malloc_framework/memoryPool.hpp | 2 + .../memoryPoolAPI.cpp | 1 + .../umf_pools/disjoint_pool.cpp | 3 +- 10 files changed, 268 insertions(+), 118 deletions(-) create mode 100644 source/common/unified_malloc_framework/src/memory_pool_default.c create mode 100644 source/common/unified_malloc_framework/src/memory_pool_internal.h create mode 100644 source/common/unified_malloc_framework/src/memory_pool_tracking.c diff --git a/CMakeLists.txt b/CMakeLists.txt index fa5f48ea77..84404d1108 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -34,6 +34,7 @@ option(UMF_BUILD_SHARED_LIBRARY "Build UMF as shared library" OFF) option(UR_ENABLE_TRACING "enable api tracing through xpti" OFF) option(VAL_USE_LIBBACKTRACE_BACKTRACE "enable libbacktrace validation backtrace for linux" OFF) option(UR_BUILD_TOOLS "build ur tools" ON) +option(UMF_ENABLE_POOL_TRACKING "Build UMF with pool tracking" OFF) set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib) set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib) diff --git a/source/common/unified_malloc_framework/CMakeLists.txt b/source/common/unified_malloc_framework/CMakeLists.txt index e73374d4eb..15744605ec 100644 --- a/source/common/unified_malloc_framework/CMakeLists.txt +++ b/source/common/unified_malloc_framework/CMakeLists.txt @@ -22,6 +22,12 @@ else() ${UMF_SOURCES}) endif() +if (UMF_ENABLE_POOL_TRACKING) + target_sources(unified_malloc_framework PRIVATE src/memory_pool_tracking.c) +else() + target_sources(unified_malloc_framework PRIVATE src/memory_pool_default.c) +endif() + add_library(${PROJECT_NAME}::unified_malloc_framework ALIAS unified_malloc_framework) target_include_directories(unified_malloc_framework PUBLIC include) diff --git a/source/common/unified_malloc_framework/src/memory_pool.c b/source/common/unified_malloc_framework/src/memory_pool.c index 4a1d16b348..7b88b2f7d6 100644 --- a/source/common/unified_malloc_framework/src/memory_pool.c +++ b/source/common/unified_malloc_framework/src/memory_pool.c @@ -8,8 +8,7 @@ * */ -#include "memory_provider_internal.h" -#include "memory_tracker.h" +#include "memory_pool_internal.h" #include #include @@ -17,87 +16,6 @@ #include #include -struct umf_memory_pool_t { - void *pool_priv; - struct umf_memory_pool_ops_t ops; - - // Holds array of memory providers. All providers are wrapped - // by memory tracking providers (owned and released by UMF). - umf_memory_provider_handle_t *providers; - - size_t numProviders; -}; - -static void -destroyMemoryProviderWrappers(umf_memory_provider_handle_t *providers, - size_t numProviders) { - for (size_t i = 0; i < numProviders; i++) { - umfMemoryProviderDestroy(providers[i]); - } - - free(providers); -} - -enum umf_result_t umfPoolCreate(const struct umf_memory_pool_ops_t *ops, - umf_memory_provider_handle_t *providers, - size_t numProviders, void *params, - umf_memory_pool_handle_t *hPool) { - if (!numProviders || !providers) { - return UMF_RESULT_ERROR_INVALID_ARGUMENT; - } - - enum umf_result_t ret = UMF_RESULT_SUCCESS; - umf_memory_pool_handle_t pool = malloc(sizeof(struct umf_memory_pool_t)); - if (!pool) { - return UMF_RESULT_ERROR_OUT_OF_HOST_MEMORY; - } - - assert(ops->version == UMF_VERSION_CURRENT); - - pool->providers = - calloc(numProviders, sizeof(umf_memory_provider_handle_t)); - if (!pool->providers) { - ret = UMF_RESULT_ERROR_OUT_OF_HOST_MEMORY; - goto err_providers_alloc; - } - - size_t providerInd = 0; - pool->numProviders = numProviders; - - // Wrap each provider with memory tracking provider. - for (providerInd = 0; providerInd < numProviders; providerInd++) { - ret = umfTrackingMemoryProviderCreate(providers[providerInd], pool, - &pool->providers[providerInd]); - if (ret != UMF_RESULT_SUCCESS) { - goto err_providers_init; - } - } - - pool->ops = *ops; - ret = ops->initialize(pool->providers, pool->numProviders, params, - &pool->pool_priv); - if (ret != UMF_RESULT_SUCCESS) { - goto err_pool_init; - } - - *hPool = pool; - return UMF_RESULT_SUCCESS; - -err_pool_init: -err_providers_init: - destroyMemoryProviderWrappers(pool->providers, providerInd); -err_providers_alloc: - free(pool); - - return ret; -} - -void umfPoolDestroy(umf_memory_pool_handle_t hPool) { - hPool->ops.finalize(hPool->pool_priv); - destroyMemoryProviderWrappers(hPool->providers, hPool->numProviders); - free(hPool); -} - void *umfPoolMalloc(umf_memory_pool_handle_t hPool, size_t size) { return hPool->ops.malloc(hPool->pool_priv, size); } @@ -123,41 +41,7 @@ enum umf_result_t umfPoolFree(umf_memory_pool_handle_t hPool, void *ptr) { return hPool->ops.free(hPool->pool_priv, ptr); } -enum umf_result_t umfFree(void *ptr) { - umf_memory_pool_handle_t hPool = umfPoolByPtr(ptr); - if (hPool) { - return umfPoolFree(hPool, ptr); - } - return UMF_RESULT_SUCCESS; -} - enum umf_result_t umfPoolGetLastAllocationError(umf_memory_pool_handle_t hPool) { return hPool->ops.get_last_allocation_error(hPool->pool_priv); } - -umf_memory_pool_handle_t umfPoolByPtr(const void *ptr) { - return umfMemoryTrackerGetPool(umfMemoryTrackerGet(), ptr); -} - -enum umf_result_t -umfPoolGetMemoryProviders(umf_memory_pool_handle_t hPool, size_t numProviders, - umf_memory_provider_handle_t *hProviders, - size_t *numProvidersRet) { - if (hProviders && numProviders < hPool->numProviders) { - return UMF_RESULT_ERROR_INVALID_ARGUMENT; - } - - if (numProvidersRet) { - *numProvidersRet = hPool->numProviders; - } - - if (hProviders) { - for (size_t i = 0; i < hPool->numProviders; i++) { - umfTrackingMemoryProviderGetUpstreamProvider( - umfMemoryProviderGetPriv(hPool->providers[i]), hProviders + i); - } - } - - return UMF_RESULT_SUCCESS; -} diff --git a/source/common/unified_malloc_framework/src/memory_pool_default.c b/source/common/unified_malloc_framework/src/memory_pool_default.c new file mode 100644 index 0000000000..2aaa97463a --- /dev/null +++ b/source/common/unified_malloc_framework/src/memory_pool_default.c @@ -0,0 +1,93 @@ +/* + * + * 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 "memory_pool_internal.h" + +#include + +#include +#include + +enum umf_result_t umfPoolCreate(const struct umf_memory_pool_ops_t *ops, + umf_memory_provider_handle_t *providers, + size_t numProviders, void *params, + umf_memory_pool_handle_t *hPool) { + if (!numProviders || !providers) { + return UMF_RESULT_ERROR_INVALID_ARGUMENT; + } + + enum umf_result_t ret = UMF_RESULT_SUCCESS; + umf_memory_pool_handle_t pool = malloc(sizeof(struct umf_memory_pool_t)); + if (!pool) { + return UMF_RESULT_ERROR_OUT_OF_HOST_MEMORY; + } + + assert(ops->version == UMF_VERSION_CURRENT); + + pool->providers = + calloc(numProviders, sizeof(umf_memory_provider_handle_t)); + if (!pool->providers) { + ret = UMF_RESULT_ERROR_OUT_OF_HOST_MEMORY; + goto err_providers_alloc; + } + + size_t providerInd = 0; + pool->numProviders = numProviders; + + for (providerInd = 0; providerInd < numProviders; providerInd++) { + pool->providers[providerInd] = providers[providerInd]; + } + + pool->ops = *ops; + ret = ops->initialize(pool->providers, pool->numProviders, params, + &pool->pool_priv); + if (ret != UMF_RESULT_SUCCESS) { + goto err_pool_init; + } + + *hPool = pool; + return UMF_RESULT_SUCCESS; + +err_pool_init: +err_providers_alloc: + free(pool); + + return ret; +} + +void umfPoolDestroy(umf_memory_pool_handle_t hPool) { + hPool->ops.finalize(hPool->pool_priv); + free(hPool); +} + +enum umf_result_t umfFree(void *ptr) { return UMF_RESULT_ERROR_NOT_SUPPORTED; } + +umf_memory_pool_handle_t umfPoolByPtr(const void *ptr) { return NULL; } + +enum umf_result_t +umfPoolGetMemoryProviders(umf_memory_pool_handle_t hPool, size_t numProviders, + umf_memory_provider_handle_t *hProviders, + size_t *numProvidersRet) { + if (hProviders && numProviders < hPool->numProviders) { + return UMF_RESULT_ERROR_INVALID_ARGUMENT; + } + + if (numProvidersRet) { + *numProvidersRet = hPool->numProviders; + } + + if (hProviders) { + for (size_t i = 0; i < hPool->numProviders; i++) { + hProviders[i] = hPool->providers[i]; + } + } + + return UMF_RESULT_SUCCESS; +} diff --git a/source/common/unified_malloc_framework/src/memory_pool_internal.h b/source/common/unified_malloc_framework/src/memory_pool_internal.h new file mode 100644 index 0000000000..37424fd3bf --- /dev/null +++ b/source/common/unified_malloc_framework/src/memory_pool_internal.h @@ -0,0 +1,37 @@ +/* + * + * 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 + * + */ + +#ifndef UMF_MEMORY_POOL_INTERNAL_H +#define UMF_MEMORY_POOL_INTERNAL_H 1 + +#include +#include +#include + +#ifdef __cplusplus +extern "C" { +#endif + +struct umf_memory_pool_t { + void *pool_priv; + struct umf_memory_pool_ops_t ops; + + // Holds array of memory providers. All providers are wrapped + // by memory tracking providers (owned and released by UMF). + umf_memory_provider_handle_t *providers; + + size_t numProviders; +}; + +#ifdef __cplusplus +} +#endif + +#endif /* UMF_MEMORY_POOL_INTERNAL_H */ diff --git a/source/common/unified_malloc_framework/src/memory_pool_tracking.c b/source/common/unified_malloc_framework/src/memory_pool_tracking.c new file mode 100644 index 0000000000..033a390fcb --- /dev/null +++ b/source/common/unified_malloc_framework/src/memory_pool_tracking.c @@ -0,0 +1,122 @@ +/* + * + * 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 "memory_pool_internal.h" +#include "memory_provider_internal.h" +#include "memory_tracker.h" + +#include + +#include +#include + +static void +destroyMemoryProviderWrappers(umf_memory_provider_handle_t *providers, + size_t numProviders) { + for (size_t i = 0; i < numProviders; i++) { + umfMemoryProviderDestroy(providers[i]); + } + + free(providers); +} + +enum umf_result_t umfPoolCreate(const struct umf_memory_pool_ops_t *ops, + umf_memory_provider_handle_t *providers, + size_t numProviders, void *params, + umf_memory_pool_handle_t *hPool) { + if (!numProviders || !providers) { + return UMF_RESULT_ERROR_INVALID_ARGUMENT; + } + + enum umf_result_t ret = UMF_RESULT_SUCCESS; + umf_memory_pool_handle_t pool = malloc(sizeof(struct umf_memory_pool_t)); + if (!pool) { + return UMF_RESULT_ERROR_OUT_OF_HOST_MEMORY; + } + + assert(ops->version == UMF_VERSION_CURRENT); + + pool->providers = + calloc(numProviders, sizeof(umf_memory_provider_handle_t)); + if (!pool->providers) { + ret = UMF_RESULT_ERROR_OUT_OF_HOST_MEMORY; + goto err_providers_alloc; + } + + size_t providerInd = 0; + pool->numProviders = numProviders; + + // Wrap each provider with memory tracking provider. + for (providerInd = 0; providerInd < numProviders; providerInd++) { + ret = umfTrackingMemoryProviderCreate(providers[providerInd], pool, + &pool->providers[providerInd]); + if (ret != UMF_RESULT_SUCCESS) { + goto err_providers_init; + } + } + + pool->ops = *ops; + ret = ops->initialize(pool->providers, pool->numProviders, params, + &pool->pool_priv); + if (ret != UMF_RESULT_SUCCESS) { + goto err_pool_init; + } + + *hPool = pool; + return UMF_RESULT_SUCCESS; + +err_pool_init: +err_providers_init: + destroyMemoryProviderWrappers(pool->providers, providerInd); +err_providers_alloc: + free(pool); + + return ret; +} + +void umfPoolDestroy(umf_memory_pool_handle_t hPool) { + hPool->ops.finalize(hPool->pool_priv); + destroyMemoryProviderWrappers(hPool->providers, hPool->numProviders); + free(hPool); +} + +enum umf_result_t umfFree(void *ptr) { + umf_memory_pool_handle_t hPool = umfPoolByPtr(ptr); + if (hPool) { + return umfPoolFree(hPool, ptr); + } + return UMF_RESULT_SUCCESS; +} + +umf_memory_pool_handle_t umfPoolByPtr(const void *ptr) { + return umfMemoryTrackerGetPool(umfMemoryTrackerGet(), ptr); +} + +enum umf_result_t +umfPoolGetMemoryProviders(umf_memory_pool_handle_t hPool, size_t numProviders, + umf_memory_provider_handle_t *hProviders, + size_t *numProvidersRet) { + if (hProviders && numProviders < hPool->numProviders) { + return UMF_RESULT_ERROR_INVALID_ARGUMENT; + } + + if (numProvidersRet) { + *numProvidersRet = hPool->numProviders; + } + + if (hProviders) { + for (size_t i = 0; i < hPool->numProviders; i++) { + umfTrackingMemoryProviderGetUpstreamProvider( + umfMemoryProviderGetPriv(hPool->providers[i]), hProviders + i); + } + } + + return UMF_RESULT_SUCCESS; +} diff --git a/test/unified_malloc_framework/CMakeLists.txt b/test/unified_malloc_framework/CMakeLists.txt index d49082c927..386ea5bdf9 100644 --- a/test/unified_malloc_framework/CMakeLists.txt +++ b/test/unified_malloc_framework/CMakeLists.txt @@ -28,6 +28,9 @@ function(add_umf_test name) COMMAND ${TEST_TARGET_NAME} WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}) set_tests_properties(umf-${name} PROPERTIES LABELS "umf") + if (UMF_ENABLE_POOL_TRACKING) + target_compile_definitions(${TEST_TARGET_NAME} PRIVATE UMF_ENABLE_POOL_TRACKING_TESTS=1) + endif() endfunction() add_subdirectory(umf_pools) diff --git a/test/unified_malloc_framework/memoryPool.hpp b/test/unified_malloc_framework/memoryPool.hpp index 813a372b25..006c4b8aed 100644 --- a/test/unified_malloc_framework/memoryPool.hpp +++ b/test/unified_malloc_framework/memoryPool.hpp @@ -251,6 +251,7 @@ TEST_P(umfPoolTest, multiThreadedMallocFreeRandomSizes) { } } +#ifdef UMF_ENABLE_POOL_TRACKING_TESTS // TODO: add similar tests for realloc/aligned_alloc, etc. // TODO: add multithreaded tests TEST_P(umfMultiPoolTest, memoryTracking) { @@ -288,5 +289,6 @@ TEST_P(umfMultiPoolTest, memoryTracking) { umfFree(std::get<0>(p)); } } +#endif /* UMF_ENABLE_POOL_TRACKING_TESTS */ #endif /* UMF_TEST_MEMORY_POOL_OPS_HPP */ diff --git a/test/unified_malloc_framework/memoryPoolAPI.cpp b/test/unified_malloc_framework/memoryPoolAPI.cpp index cfbe068719..99769acab0 100644 --- a/test/unified_malloc_framework/memoryPoolAPI.cpp +++ b/test/unified_malloc_framework/memoryPoolAPI.cpp @@ -148,6 +148,7 @@ INSTANTIATE_TEST_SUITE_P( .second; })); +GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(umfMultiPoolTest); INSTANTIATE_TEST_SUITE_P( mallocMultiPoolTest, umfMultiPoolTest, ::testing::Values([] { return umf::poolMakeUnique( diff --git a/test/unified_malloc_framework/umf_pools/disjoint_pool.cpp b/test/unified_malloc_framework/umf_pools/disjoint_pool.cpp index 5c9584e46d..9e4d4f7ee6 100644 --- a/test/unified_malloc_framework/umf_pools/disjoint_pool.cpp +++ b/test/unified_malloc_framework/umf_pools/disjoint_pool.cpp @@ -64,7 +64,7 @@ TEST_F(test, freeErrorPropagation) { void *ptr = umfPoolMalloc(pool.get(), size); freeReturn = UMF_RESULT_ERROR_MEMORY_PROVIDER_SPECIFIC; - auto freeRet = umfFree(ptr); + auto freeRet = umfPoolFree(pool.get(), ptr); EXPECT_EQ(freeRet, freeReturn); } @@ -72,5 +72,6 @@ TEST_F(test, freeErrorPropagation) { INSTANTIATE_TEST_SUITE_P(disjointPoolTests, umfPoolTest, ::testing::Values(makePool)); +GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(umfMultiPoolTest); INSTANTIATE_TEST_SUITE_P(disjointMultiPoolTests, umfMultiPoolTest, ::testing::Values(makePool)); From ff705f5d4f9651faa21aacb604dc4fe8947bd2b7 Mon Sep 17 00:00:00 2001 From: Krzysztof Swiecicki Date: Mon, 24 Jul 2023 15:27:36 +0200 Subject: [PATCH 2/2] Enable UMF pool tracking in github workflows --- .github/workflows/cmake.yml | 7 +++++-- .github/workflows/codeql.yml | 4 ++-- .github/workflows/coverity.yml | 6 +++--- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/.github/workflows/cmake.yml b/.github/workflows/cmake.yml index d3fd0e3e5e..8f32e6058b 100644 --- a/.github/workflows/cmake.yml +++ b/.github/workflows/cmake.yml @@ -11,6 +11,7 @@ jobs: build_type: [Debug, Release] compiler: [{c: gcc, cxx: g++}] libbacktrace: ['-DVAL_USE_LIBBACKTRACE_BACKTRACE=OFF'] + pool_tracking: ['-DUMF_ENABLE_POOL_TRACKING=ON', '-DUMF_ENABLE_POOL_TRACKING=OFF'] include: - os: 'ubuntu-22.04' build_type: Release @@ -53,9 +54,9 @@ jobs: cd libbacktrace ./configure make - sudo make install + sudo make install cd .. - + - name: Download DPC++ run: | sudo apt install libncurses5 @@ -75,6 +76,7 @@ jobs: -DUR_FORMAT_CPP_STYLE=ON -DUR_DPCXX=${{github.workspace}}/dpcpp_compiler/bin/clang++ ${{matrix.libbacktrace}} + ${{matrix.pool_tracking}} - name: Generate source from spec, check for uncommitted diff if: matrix.os == 'ubuntu-22.04' @@ -153,5 +155,6 @@ jobs: -DCMAKE_BUILD_TYPE=Release -DUR_BUILD_TESTS=ON -DUR_FORMAT_CPP_STYLE=ON + -DUMF_ENABLE_POOL_TRACKING=ON - name: Build run: cmake --build ${{github.workspace}}/build -j $(nproc) diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 31bf8d2515..701a3be4a2 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -27,7 +27,7 @@ jobs: run: pip install -r third_party/requirements.txt - name: Configure CMake - run: cmake -B ${{github.workspace}}/build -DUR_DEVELOPER_MODE=ON -DUR_BUILD_TESTS=ON -DUR_ENABLE_TRACING=ON -DUR_BUILD_TOOLS=ON + run: cmake -B ${{github.workspace}}/build -DUR_DEVELOPER_MODE=ON -DUR_BUILD_TESTS=ON -DUR_ENABLE_TRACING=ON -DUR_BUILD_TOOLS=ON -DUMF_ENABLE_POOL_TRACKING=ON - name: Build run: cmake --build ${{github.workspace}}/build -j $(nproc) @@ -61,7 +61,7 @@ jobs: run: python3 -m pip install -r third_party/requirements.txt - name: Configure CMake - run: cmake -B ${{github.workspace}}/build -DCMAKE_POLICY_DEFAULT_CMP0094=NEW -DUR_DEVELOPER_MODE=ON -DUR_BUILD_TESTS=ON -DUR_ENABLE_TRACING=ON -DUR_BUILD_TOOLS=ON + run: cmake -B ${{github.workspace}}/build -DCMAKE_POLICY_DEFAULT_CMP0094=NEW -DUR_DEVELOPER_MODE=ON -DUR_BUILD_TESTS=ON -DUR_ENABLE_TRACING=ON -DUR_BUILD_TOOLS=ON -DUMF_ENABLE_POOL_TRACKING=ON - name: Build run: cmake --build ${{github.workspace}}/build -j $(nproc) --config Release diff --git a/.github/workflows/coverity.yml b/.github/workflows/coverity.yml index 7d825e92a7..ed6cd31ca9 100644 --- a/.github/workflows/coverity.yml +++ b/.github/workflows/coverity.yml @@ -26,7 +26,7 @@ jobs: linux: name: Coverity runs-on: ubuntu-latest - + steps: - name: Clone the git repo uses: actions/checkout@v3 @@ -35,12 +35,12 @@ jobs: run: | sudo apt-get update sudo apt-get install -y doxygen - + - name: Install pip packages run: pip install -r third_party/requirements.txt - name: Configure CMake - run: cmake -B $WORKDIR/build -DUR_ENABLE_TRACING=ON -DUR_DEVELOPER_MODE=ON -DUR_BUILD_TESTS=ON -DUR_FORMAT_CPP_STYLE=ON + run: cmake -B $WORKDIR/build -DUR_ENABLE_TRACING=ON -DUR_DEVELOPER_MODE=ON -DUR_BUILD_TESTS=ON -DUR_FORMAT_CPP_STYLE=ON -DUMF_ENABLE_POOL_TRACKING=ON - name: Generate source from spec, check for uncommitted diff run: |