From fa8ec81ca519c4f2303cd3655ec206860b6d275f Mon Sep 17 00:00:00 2001 From: Igor Chorazewicz Date: Tue, 20 Jun 2023 16:03:22 +0000 Subject: [PATCH 1/4] [uma] Improve reporting of pool/provider specific errors Memory provider: Remove the get_last_result function and replace it with get_last_error. This primary difference is the addition of the pError out parameter which returns a native error code emitted from a failed provider function which resulted in returning UMA_RESULT_ERROR_MEMORY_PROVIDER_SPECIFIC. Memory_pool: Remove the get_last_result function and replace it with get_last_allocation_status. For memory pool, I don't think we care about a pool-specific error codes. We do, however, want to know why the previous allocation/de-allocation failed. Memory pool allocation functions only return a pointer so we need an extra function (get_last_allocation_status) to get this information. --- source/common/uma_helpers.hpp | 5 +- source/common/uma_pools/disjoint_pool.cpp | 10 ++-- source/common/uma_pools/disjoint_pool.hpp | 2 +- .../include/uma/base.h | 13 ++--- .../include/uma/memory_pool.h | 33 +++++------- .../include/uma/memory_pool_ops.h | 2 +- .../include/uma/memory_provider.h | 28 +++++----- .../include/uma/memory_provider_ops.h | 4 +- .../src/memory_pool.c | 6 +-- .../src/memory_provider.c | 9 ++-- .../src/memory_tracker.cpp | 8 +-- test/unified_memory_allocation/common/pool.c | 53 +++++++++---------- .../unified_memory_allocation/common/pool.hpp | 7 +-- .../common/provider.c | 19 +++---- .../common/provider.hpp | 4 +- .../memoryPoolAPI.cpp | 9 +--- .../memoryProviderAPI.cpp | 7 ++- 17 files changed, 100 insertions(+), 119 deletions(-) diff --git a/source/common/uma_helpers.hpp b/source/common/uma_helpers.hpp index 67eddd6811..35f4123491 100644 --- a/source/common/uma_helpers.hpp +++ b/source/common/uma_helpers.hpp @@ -87,7 +87,7 @@ auto memoryProviderMakeUnique(Args &&...args) { UMA_ASSIGN_OP(ops, T, alloc, UMA_RESULT_ERROR_UNKNOWN); UMA_ASSIGN_OP(ops, T, free, UMA_RESULT_ERROR_UNKNOWN); - UMA_ASSIGN_OP(ops, T, get_last_result, UMA_RESULT_ERROR_UNKNOWN); + UMA_ASSIGN_OP_NORETURN(ops, T, get_last_native_error); UMA_ASSIGN_OP(ops, T, get_recommended_page_size, UMA_RESULT_ERROR_UNKNOWN); UMA_ASSIGN_OP(ops, T, get_min_page_size, UMA_RESULT_ERROR_UNKNOWN); UMA_ASSIGN_OP(ops, T, purge_lazy, UMA_RESULT_ERROR_UNKNOWN); @@ -147,13 +147,14 @@ auto poolMakeUnique(uma_memory_provider_handle_t *providers, UMA_ASSIGN_OP(ops, T, realloc, ((void *)nullptr)); UMA_ASSIGN_OP(ops, T, malloc_usable_size, ((size_t)0)); UMA_ASSIGN_OP_NORETURN(ops, T, free); - UMA_ASSIGN_OP(ops, T, get_last_result, UMA_RESULT_ERROR_UNKNOWN); + UMA_ASSIGN_OP(ops, T, get_last_allocation_error, UMA_RESULT_ERROR_UNKNOWN); uma_memory_pool_handle_t hPool = nullptr; auto ret = umaPoolCreate(&ops, providers, numProviders, &argsTuple, &hPool); return std::pair{ ret, pool_unique_handle_t(hPool, &umaPoolDestroy)}; } + } // namespace uma #endif /* UMA_HELPERS_H */ diff --git a/source/common/uma_pools/disjoint_pool.cpp b/source/common/uma_pools/disjoint_pool.cpp index 562d8f1d7c..c26e654da7 100644 --- a/source/common/uma_pools/disjoint_pool.cpp +++ b/source/common/uma_pools/disjoint_pool.cpp @@ -332,8 +332,7 @@ static void *memoryProviderAlloc(uma_memory_provider_handle_t hProvider, void *ptr; auto ret = umaMemoryProviderAlloc(hProvider, size, alignment, &ptr); if (ret != UMA_RESULT_SUCCESS) { - // TODO: Set last error appropriately - throw std::runtime_error("umaMemoryProviderAlloc"); + throw MemoryProviderError{ret}; } return ptr; } @@ -342,8 +341,7 @@ static void memoryProviderFree(uma_memory_provider_handle_t hProvider, void *ptr) { auto ret = umaMemoryProviderFree(hProvider, ptr, 0); if (ret != UMA_RESULT_SUCCESS) { - // TODO: introduce custom exceptions? PI L0 relies on those - throw std::runtime_error("memoryProviderFree"); + throw MemoryProviderError{ret}; } } @@ -939,9 +937,7 @@ void DisjointPool::free(void *ptr) { return; } -enum uma_result_t DisjointPool::get_last_result(const char **ppMessage) { - // TODO: implement and return last error, we probably need something like - // https://github.com/oneapi-src/unified-runtime/issues/500 in UMA +enum uma_result_t DisjointPool::get_last_allocation_error() { return UMA_RESULT_ERROR_UNKNOWN; } diff --git a/source/common/uma_pools/disjoint_pool.hpp b/source/common/uma_pools/disjoint_pool.hpp index c991fa07bb..fb1e34f2d7 100644 --- a/source/common/uma_pools/disjoint_pool.hpp +++ b/source/common/uma_pools/disjoint_pool.hpp @@ -68,7 +68,7 @@ class DisjointPool { void *aligned_malloc(size_t size, size_t alignment); size_t malloc_usable_size(void *); void free(void *ptr); - enum uma_result_t get_last_result(const char **ppMessage); + enum uma_result_t get_last_allocation_error(); DisjointPool(); ~DisjointPool(); diff --git a/source/common/unified_memory_allocation/include/uma/base.h b/source/common/unified_memory_allocation/include/uma/base.h index 7d5d34f987..bc00e2db3a 100644 --- a/source/common/unified_memory_allocation/include/uma/base.h +++ b/source/common/unified_memory_allocation/include/uma/base.h @@ -36,16 +36,13 @@ enum uma_result_t { UMA_RESULT_SUCCESS = 0, ///< Success UMA_RESULT_ERROR_OUT_OF_HOST_MEMORY = 1, ///< Insufficient host memory to satisfy call, - UMA_RESULT_ERROR_POOL_SPECIFIC = - 2, ///< A pool specific warning/error has been reported and can be - ///< Retrieved via the umaPoolGetLastResult entry point. UMA_RESULT_ERROR_MEMORY_PROVIDER_SPECIFIC = - 3, ///< A provider specific warning/error has been reported and can be - ///< Retrieved via the umaMemoryProviderGetLastResult entry point. + 2, ///< A provider specific warning/error has been reported and can be + ///< Retrieved via the umaMemoryProviderGetLastNativeError entry point. UMA_RESULT_ERROR_INVALID_ARGUMENT = - 4, ///< Generic error code for invalid arguments - UMA_RESULT_ERROR_INVALID_ALIGNMENT = 5, /// Invalid alignment of an argument - UMA_RESULT_ERROR_NOT_SUPPORTED = 6, /// Operation not supported + 3, ///< Generic error code for invalid arguments + UMA_RESULT_ERROR_INVALID_ALIGNMENT = 4, /// Invalid alignment of an argument + UMA_RESULT_ERROR_NOT_SUPPORTED = 5, /// Operation not supported UMA_RESULT_ERROR_UNKNOWN = 0x7ffffffe ///< Unknown or internal error }; diff --git a/source/common/unified_memory_allocation/include/uma/memory_pool.h b/source/common/unified_memory_allocation/include/uma/memory_pool.h index d2cc044a58..812aa0c963 100644 --- a/source/common/unified_memory_allocation/include/uma/memory_pool.h +++ b/source/common/unified_memory_allocation/include/uma/memory_pool.h @@ -104,28 +104,23 @@ void umaPoolFree(uma_memory_pool_handle_t hPool, void *ptr); void umaFree(void *ptr); /// -/// \brief Retrieve string representation of the underlying pool specific -/// result reported by the last API that returned -/// UMA_RESULT_ERROR_POOL_SPECIFIC or NULL ptr (in case of allocation -/// functions). Allows for a pool independent way to -/// return a pool specific result. +/// \brief Retrieve uma_result_t representing the error of the last failed allocation +/// operation in this thread (malloc, calloc, realloc, aligned_malloc). /// /// \details -/// - The string returned via the ppMessage is a NULL terminated C style -/// string. -/// - The string returned via the ppMessage is thread local. -/// - The memory in the string returned via the ppMessage is owned by the -/// adapter. -/// - The application may call this function from simultaneous threads. -/// - The implementation of this function should be lock-free. +/// * Implementations *must* store the error code in thread-local +/// storage prior to returning NULL from the allocation functions. +/// +/// * If the last allocation/de-allocation operation succeeded, the value returned by +/// this function is unspecified. +/// +/// * The application *may* call this function from simultaneous threads. +/// +/// * The implementation of this function *should* be lock-free. /// \param hPool specified memory hPool -/// \param ppMessage [out] pointer to a string containing provider specific -/// result in string representation. -/// \return UMA_RESULT_SUCCESS if the result being -/// reported is to be considered a warning. Any other result code -/// returned indicates that the adapter specific result is an error. -enum uma_result_t umaPoolGetLastResult(uma_memory_pool_handle_t hPool, - const char **ppMessage); +/// \return Error code desciribng the failure of the last failed allocation operation. +/// The value is undefined if the previous allocation was successful. +enum uma_result_t umaPoolGetLastAllocationError(uma_memory_pool_handle_t hPool); /// /// \brief Retrieve memory pool associated with a given ptr. diff --git a/source/common/unified_memory_allocation/include/uma/memory_pool_ops.h b/source/common/unified_memory_allocation/include/uma/memory_pool_ops.h index 31a15ca02f..21fae70b27 100644 --- a/source/common/unified_memory_allocation/include/uma/memory_pool_ops.h +++ b/source/common/unified_memory_allocation/include/uma/memory_pool_ops.h @@ -49,7 +49,7 @@ struct uma_memory_pool_ops_t { void *(*aligned_malloc)(void *pool, size_t size, size_t alignment); size_t (*malloc_usable_size)(void *pool, void *ptr); void (*free)(void *pool, void *); - enum uma_result_t (*get_last_result)(void *pool, const char **ppMessage); + enum uma_result_t (*get_last_allocation_error)(void *pool); }; #ifdef __cplusplus diff --git a/source/common/unified_memory_allocation/include/uma/memory_provider.h b/source/common/unified_memory_allocation/include/uma/memory_provider.h index d3c2df3ac4..8201d54f22 100644 --- a/source/common/unified_memory_allocation/include/uma/memory_provider.h +++ b/source/common/unified_memory_allocation/include/uma/memory_provider.h @@ -67,23 +67,25 @@ enum uma_result_t umaMemoryProviderFree(uma_memory_provider_handle_t hProvider, /// independent way to return a provider specific result. /// /// \details -/// - The string returned via the ppMessage is a NULL terminated C style -/// string. -/// - The string returned via the ppMessage is thread local. -/// - The memory in the string returned via the ppMessage is owned by the -/// adapter. -/// - The application may call this function from simultaneous threads. -/// - The implementation of this function should be lock-free. +/// * Implementations *must* store the message and error code in thread-local +/// storage prior to returning UMA_RESULT_ERROR_MEMORY_PROVIDER_SPECIFIC. /// +/// * The message and error code will only be valid if a previously +/// called entry-point returned UMA_RESULT_ERROR_MEMORY_PROVIDER_SPECIFIC. +/// +/// * The memory pointed to by the C string returned in `ppMessage` is owned by +/// the adapter and *must* be null terminated. +/// +/// * The application *may* call this function from simultaneous threads. +/// +/// * The implementation of this function *should* be lock-free. /// \param hProvider handle to the memory provider /// \param ppMessage [out] pointer to a string containing provider specific /// result in string representation -/// \return UMA_RESULT_SUCCESS if the result being reported is to be considered -/// a warning. Any other result code returned indicates that the -/// adapter specific result is an error. -enum uma_result_t -umaMemoryProviderGetLastResult(uma_memory_provider_handle_t hProvider, - const char **ppMessage); +/// \param pError [out] pointer to an integer where the adapter specific error code will be stored +void umaMemoryProviderGetLastNativeError(uma_memory_provider_handle_t hProvider, + const char **ppMessage, + int32_t *pError); /// /// \brief Retrieve recommended page size for a given allocation size. diff --git a/source/common/unified_memory_allocation/include/uma/memory_provider_ops.h b/source/common/unified_memory_allocation/include/uma/memory_provider_ops.h index 40c2ac8a0f..e5afbdaa53 100644 --- a/source/common/unified_memory_allocation/include/uma/memory_provider_ops.h +++ b/source/common/unified_memory_allocation/include/uma/memory_provider_ops.h @@ -41,8 +41,8 @@ struct uma_memory_provider_ops_t { enum uma_result_t (*alloc)(void *provider, size_t size, size_t alignment, void **ptr); enum uma_result_t (*free)(void *provider, void *ptr, size_t size); - enum uma_result_t (*get_last_result)(void *provider, - const char **ppMessage); + void (*get_last_native_error)(void *provider, const char **ppMessage, + int32_t *pError); enum uma_result_t (*get_recommended_page_size)(void *provider, size_t size, size_t *pageSize); enum uma_result_t (*get_min_page_size)(void *provider, void *ptr, diff --git a/source/common/unified_memory_allocation/src/memory_pool.c b/source/common/unified_memory_allocation/src/memory_pool.c index 754e4697bd..13a86d8a6c 100644 --- a/source/common/unified_memory_allocation/src/memory_pool.c +++ b/source/common/unified_memory_allocation/src/memory_pool.c @@ -130,9 +130,9 @@ void umaFree(void *ptr) { } } -enum uma_result_t umaPoolGetLastResult(uma_memory_pool_handle_t hPool, - const char **ppMessage) { - return hPool->ops.get_last_result(hPool->pool_priv, ppMessage); +enum uma_result_t +umaPoolGetLastAllocationError(uma_memory_pool_handle_t hPool) { + return hPool->ops.get_last_allocation_error(hPool->pool_priv); } uma_memory_pool_handle_t umaPoolByPtr(const void *ptr) { diff --git a/source/common/unified_memory_allocation/src/memory_provider.c b/source/common/unified_memory_allocation/src/memory_provider.c index 23b05cd4d4..345bc8e6ab 100644 --- a/source/common/unified_memory_allocation/src/memory_provider.c +++ b/source/common/unified_memory_allocation/src/memory_provider.c @@ -62,10 +62,11 @@ enum uma_result_t umaMemoryProviderFree(uma_memory_provider_handle_t hProvider, return hProvider->ops.free(hProvider->provider_priv, ptr, size); } -enum uma_result_t -umaMemoryProviderGetLastResult(uma_memory_provider_handle_t hProvider, - const char **ppMessage) { - return hProvider->ops.get_last_result(hProvider->provider_priv, ppMessage); +void umaMemoryProviderGetLastNativeError(uma_memory_provider_handle_t hProvider, + const char **ppMessage, + int32_t *pError) { + hProvider->ops.get_last_native_error(hProvider->provider_priv, ppMessage, + pError); } void *umaMemoryProviderGetPriv(uma_memory_provider_handle_t hProvider) { diff --git a/source/common/unified_memory_allocation/src/memory_tracker.cpp b/source/common/unified_memory_allocation/src/memory_tracker.cpp index 6f56134db2..0dc118b738 100644 --- a/source/common/unified_memory_allocation/src/memory_tracker.cpp +++ b/source/common/unified_memory_allocation/src/memory_tracker.cpp @@ -168,11 +168,11 @@ static enum uma_result_t trackingInitialize(void *params, void **ret) { static void trackingFinalize(void *provider) { free(provider); } -static enum uma_result_t trackingGetLastResult(void *provider, - const char **msg) { +static void trackingGetLastError(void *provider, const char **msg, + int32_t *pError) { uma_tracking_memory_provider_t *p = (uma_tracking_memory_provider_t *)provider; - return umaMemoryProviderGetLastResult(p->hUpstream, msg); + umaMemoryProviderGetLastNativeError(p->hUpstream, msg, pError); } static enum uma_result_t @@ -224,7 +224,7 @@ enum uma_result_t umaTrackingMemoryProviderCreate( trackingMemoryProviderOps.finalize = trackingFinalize; trackingMemoryProviderOps.alloc = trackingAlloc; trackingMemoryProviderOps.free = trackingFree; - trackingMemoryProviderOps.get_last_result = trackingGetLastResult; + trackingMemoryProviderOps.get_last_native_error = trackingGetLastError; trackingMemoryProviderOps.get_min_page_size = trackingGetMinPageSize; trackingMemoryProviderOps.get_recommended_page_size = trackingGetRecommendedPageSize; diff --git a/test/unified_memory_allocation/common/pool.c b/test/unified_memory_allocation/common/pool.c index 7d229cf11f..200ed75dff 100644 --- a/test/unified_memory_allocation/common/pool.c +++ b/test/unified_memory_allocation/common/pool.c @@ -62,24 +62,23 @@ static void nullFree(void *pool, void *ptr) { (void)ptr; } -enum uma_result_t nullGetLastResult(void *pool, const char **ppMsg) { +enum uma_result_t nullGetLastStatus(void *pool) { (void)pool; - (void)ppMsg; return UMA_RESULT_SUCCESS; } uma_memory_pool_handle_t nullPoolCreate(void) { - struct uma_memory_pool_ops_t ops = {.version = UMA_VERSION_CURRENT, - .initialize = nullInitialize, - .finalize = nullFinalize, - .malloc = nullMalloc, - .realloc = nullRealloc, - .calloc = nullCalloc, - .aligned_malloc = nullAlignedMalloc, - .malloc_usable_size = - nullMallocUsableSize, - .free = nullFree, - .get_last_result = nullGetLastResult}; + struct uma_memory_pool_ops_t ops = { + .version = UMA_VERSION_CURRENT, + .initialize = nullInitialize, + .finalize = nullFinalize, + .malloc = nullMalloc, + .realloc = nullRealloc, + .calloc = nullCalloc, + .aligned_malloc = nullAlignedMalloc, + .malloc_usable_size = nullMallocUsableSize, + .free = nullFree, + .get_last_allocation_error = nullGetLastStatus}; uma_memory_provider_handle_t providerDesc = nullProviderCreate(); uma_memory_pool_handle_t hPool; @@ -159,28 +158,28 @@ static void traceFree(void *pool, void *ptr) { umaPoolFree(tracePool->params.hUpstreamPool, ptr); } -enum uma_result_t traceGetLastResult(void *pool, const char **ppMsg) { +enum uma_result_t traceGetLastStatus(void *pool) { struct tracePool *tracePool = (struct tracePool *)pool; - tracePool->params.trace("get_last_result"); - return umaPoolGetLastResult(tracePool->params.hUpstreamPool, ppMsg); + tracePool->params.trace("get_last_native_error"); + return umaPoolGetLastAllocationError(tracePool->params.hUpstreamPool); } uma_memory_pool_handle_t tracePoolCreate(uma_memory_pool_handle_t hUpstreamPool, uma_memory_provider_handle_t providerDesc, void (*trace)(const char *)) { - struct uma_memory_pool_ops_t ops = {.version = UMA_VERSION_CURRENT, - .initialize = traceInitialize, - .finalize = traceFinalize, - .malloc = traceMalloc, - .realloc = traceRealloc, - .calloc = traceCalloc, - .aligned_malloc = traceAlignedMalloc, - .malloc_usable_size = - traceMallocUsableSize, - .free = traceFree, - .get_last_result = traceGetLastResult}; + struct uma_memory_pool_ops_t ops = { + .version = UMA_VERSION_CURRENT, + .initialize = traceInitialize, + .finalize = traceFinalize, + .malloc = traceMalloc, + .realloc = traceRealloc, + .calloc = traceCalloc, + .aligned_malloc = traceAlignedMalloc, + .malloc_usable_size = traceMallocUsableSize, + .free = traceFree, + .get_last_allocation_error = traceGetLastStatus}; struct traceParams params = {.hUpstreamPool = hUpstreamPool, .trace = trace}; diff --git a/test/unified_memory_allocation/common/pool.hpp b/test/unified_memory_allocation/common/pool.hpp index f360af34dd..f9830f5f83 100644 --- a/test/unified_memory_allocation/common/pool.hpp +++ b/test/unified_memory_allocation/common/pool.hpp @@ -37,8 +37,8 @@ struct pool_base { void *aligned_malloc(size_t, size_t) noexcept { return nullptr; } size_t malloc_usable_size(void *) noexcept { return 0; } void free(void *) noexcept {} - enum uma_result_t get_last_result(const char **ppMessage) noexcept { - return UMA_RESULT_ERROR_UNKNOWN; + enum uma_result_t get_last_allocation_error() noexcept { + return UMA_RESULT_SUCCESS; } }; @@ -102,9 +102,6 @@ struct proxy_pool : public pool_base { auto ret = umaMemoryProviderFree(provider, ptr, 0); EXPECT_EQ_NOEXCEPT(ret, UMA_RESULT_SUCCESS); } - enum uma_result_t get_last_result(const char **ppMessage) noexcept { - return umaMemoryProviderGetLastResult(provider, ppMessage); - } uma_memory_provider_handle_t provider; }; diff --git a/test/unified_memory_allocation/common/provider.c b/test/unified_memory_allocation/common/provider.c index 05dce162ed..55968223da 100644 --- a/test/unified_memory_allocation/common/provider.c +++ b/test/unified_memory_allocation/common/provider.c @@ -34,10 +34,11 @@ static enum uma_result_t nullFree(void *provider, void *ptr, size_t size) { return UMA_RESULT_SUCCESS; } -static enum uma_result_t nullGetLastResult(void *provider, const char **ppMsg) { +static void nullGetLastError(void *provider, const char **ppMsg, + int32_t *pError) { (void)provider; (void)ppMsg; - return UMA_RESULT_SUCCESS; + (void)pError; } static enum uma_result_t nullGetRecommendedPageSize(void *provider, size_t size, @@ -84,7 +85,7 @@ uma_memory_provider_handle_t nullProviderCreate(void) { .finalize = nullFinalize, .alloc = nullAlloc, .free = nullFree, - .get_last_result = nullGetLastResult, + .get_last_native_error = nullGetLastError, .get_recommended_page_size = nullGetRecommendedPageSize, .get_min_page_size = nullGetPageSize, .purge_lazy = nullPurgeLazy, @@ -131,13 +132,13 @@ static enum uma_result_t traceFree(void *provider, void *ptr, size_t size) { return umaMemoryProviderFree(traceProvider->hUpstreamProvider, ptr, size); } -static enum uma_result_t traceGetLastResult(void *provider, - const char **ppMsg) { +static void traceGetLastError(void *provider, const char **ppMsg, + int32_t *pError) { struct traceParams *traceProvider = (struct traceParams *)provider; - traceProvider->trace("get_last_result"); - return umaMemoryProviderGetLastResult(traceProvider->hUpstreamProvider, - ppMsg); + traceProvider->trace("get_last_native_error"); + umaMemoryProviderGetLastNativeError(traceProvider->hUpstreamProvider, ppMsg, + pError); } static enum uma_result_t @@ -193,7 +194,7 @@ traceProviderCreate(uma_memory_provider_handle_t hUpstreamProvider, .finalize = traceFinalize, .alloc = traceAlloc, .free = traceFree, - .get_last_result = traceGetLastResult, + .get_last_native_error = traceGetLastError, .get_recommended_page_size = traceGetRecommendedPageSize, .get_min_page_size = traceGetPageSize, .purge_lazy = tracePurgeLazy, diff --git a/test/unified_memory_allocation/common/provider.hpp b/test/unified_memory_allocation/common/provider.hpp index c60ebf0c3d..6030990f52 100644 --- a/test/unified_memory_allocation/common/provider.hpp +++ b/test/unified_memory_allocation/common/provider.hpp @@ -33,9 +33,7 @@ struct provider_base { enum uma_result_t free(void *ptr, size_t size) noexcept { return UMA_RESULT_ERROR_UNKNOWN; } - enum uma_result_t get_last_result(const char **) noexcept { - return UMA_RESULT_ERROR_UNKNOWN; - } + void get_last_native_error(const char **, int32_t *) noexcept {} enum uma_result_t get_recommended_page_size(size_t size, size_t *pageSize) noexcept { return UMA_RESULT_ERROR_UNKNOWN; diff --git a/test/unified_memory_allocation/memoryPoolAPI.cpp b/test/unified_memory_allocation/memoryPoolAPI.cpp index af5aa5f077..ffcc3afed8 100644 --- a/test/unified_memory_allocation/memoryPoolAPI.cpp +++ b/test/unified_memory_allocation/memoryPoolAPI.cpp @@ -80,15 +80,11 @@ TEST_F(test, memoryPoolTrace) { ASSERT_EQ(providerCalls.size(), provider_call_count); - ret = umaPoolGetLastResult(tracingPool.get(), nullptr); + ret = umaPoolGetLastAllocationError(tracingPool.get()); ASSERT_EQ(ret, UMA_RESULT_SUCCESS); - - ASSERT_EQ(poolCalls["get_last_result"], 1); + ASSERT_EQ(poolCalls["get_last_native_error"], 1); ASSERT_EQ(poolCalls.size(), ++pool_call_count); - ASSERT_EQ(providerCalls["get_last_result"], 1); - ASSERT_EQ(providerCalls.size(), ++provider_call_count); - umaMemoryProviderDestroy(providerDesc); } @@ -194,7 +190,6 @@ struct poolInitializeTest : uma_test::test, INSTANTIATE_TEST_SUITE_P( poolInitializeTest, poolInitializeTest, ::testing::Values(UMA_RESULT_ERROR_OUT_OF_HOST_MEMORY, - UMA_RESULT_ERROR_POOL_SPECIFIC, UMA_RESULT_ERROR_MEMORY_PROVIDER_SPECIFIC, UMA_RESULT_ERROR_INVALID_ARGUMENT, UMA_RESULT_ERROR_UNKNOWN)); diff --git a/test/unified_memory_allocation/memoryProviderAPI.cpp b/test/unified_memory_allocation/memoryProviderAPI.cpp index f770050f32..510c2b88be 100644 --- a/test/unified_memory_allocation/memoryProviderAPI.cpp +++ b/test/unified_memory_allocation/memoryProviderAPI.cpp @@ -33,9 +33,9 @@ TEST_F(test, memoryProviderTrace) { ASSERT_EQ(calls["free"], 1); ASSERT_EQ(calls.size(), ++call_count); - ret = umaMemoryProviderGetLastResult(tracingProvider.get(), nullptr); - ASSERT_EQ(ret, UMA_RESULT_SUCCESS); - ASSERT_EQ(calls["get_last_result"], 1); + umaMemoryProviderGetLastNativeError(tracingProvider.get(), nullptr, + nullptr); + ASSERT_EQ(calls["get_last_native_error"], 1); ASSERT_EQ(calls.size(), ++call_count); ret = umaMemoryProviderGetRecommendedPageSize(tracingProvider.get(), 0, @@ -76,7 +76,6 @@ struct providerInitializeTest : uma_test::test, INSTANTIATE_TEST_SUITE_P( providerInitializeTest, providerInitializeTest, ::testing::Values(UMA_RESULT_ERROR_OUT_OF_HOST_MEMORY, - UMA_RESULT_ERROR_POOL_SPECIFIC, UMA_RESULT_ERROR_MEMORY_PROVIDER_SPECIFIC, UMA_RESULT_ERROR_INVALID_ARGUMENT, UMA_RESULT_ERROR_UNKNOWN)); From 438c0d37f8b777fb86c9ff571566526b2dddaf36 Mon Sep 17 00:00:00 2001 From: Igor Chorazewicz Date: Tue, 20 Jun 2023 17:33:44 +0000 Subject: [PATCH 2/4] [uma] Improve error reporing int DisjointPool use get_last_allocation_error to return last error from allocation functions. --- source/common/uma_helpers.hpp | 5 ++++ source/common/uma_pools/disjoint_pool.cpp | 28 +++++++++++++++++++---- 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/source/common/uma_helpers.hpp b/source/common/uma_helpers.hpp index 35f4123491..2d7d9a4059 100644 --- a/source/common/uma_helpers.hpp +++ b/source/common/uma_helpers.hpp @@ -155,6 +155,11 @@ auto poolMakeUnique(uma_memory_provider_handle_t *providers, ret, pool_unique_handle_t(hPool, &umaPoolDestroy)}; } +template uma_result_t &getPoolLastStatusRef() { + static thread_local uma_result_t last_status = UMA_RESULT_SUCCESS; + return last_status; +} + } // namespace uma #endif /* UMA_HELPERS_H */ diff --git a/source/common/uma_pools/disjoint_pool.cpp b/source/common/uma_pools/disjoint_pool.cpp index c26e654da7..34f72a290e 100644 --- a/source/common/uma_pools/disjoint_pool.cpp +++ b/source/common/uma_pools/disjoint_pool.cpp @@ -74,6 +74,10 @@ static size_t AlignUp(size_t Val, size_t Alignment) { return (Val + Alignment - 1) & (~(Alignment - 1)); } +struct MemoryProviderError { + uma_result_t code; +}; + DisjointPoolConfig::DisjointPoolConfig() : limits(std::make_shared()) {} @@ -371,7 +375,13 @@ Slab::~Slab() { } catch (std::exception &e) { std::cout << "DisjointPool: unexpected error: " << e.what() << "\n"; } - memoryProviderFree(bucket.getMemHandle(), MemPtr); + + try { + memoryProviderFree(bucket.getMemHandle(), MemPtr); + } catch (MemoryProviderError &e) { + std::cout << "DisjointPool: error from memory provider: " << e.code + << "\n"; + } } // Return the index of the first available chunk, SIZE_MAX otherwise @@ -717,7 +727,7 @@ void Bucket::printStats(bool &TitlePrinted, const std::string &Label) { } } -void *DisjointPool::AllocImpl::allocate(size_t Size, bool &FromPool) { +void *DisjointPool::AllocImpl::allocate(size_t Size, bool &FromPool) try { void *Ptr; if (Size == 0) { @@ -742,10 +752,13 @@ void *DisjointPool::AllocImpl::allocate(size_t Size, bool &FromPool) { } return Ptr; +} catch (MemoryProviderError &e) { + uma::getPoolLastStatusRef() = e.code; + return nullptr; } void *DisjointPool::AllocImpl::allocate(size_t Size, size_t Alignment, - bool &FromPool) { + bool &FromPool) try { void *Ptr; if (Size == 0) { @@ -789,6 +802,9 @@ void *DisjointPool::AllocImpl::allocate(size_t Size, size_t Alignment, } return AlignPtrUp(Ptr, Alignment); +} catch (MemoryProviderError &e) { + uma::getPoolLastStatusRef() = e.code; + return nullptr; } Bucket &DisjointPool::AllocImpl::findBucket(size_t Size) { @@ -803,7 +819,7 @@ Bucket &DisjointPool::AllocImpl::findBucket(size_t Size) { return *(*It); } -void DisjointPool::AllocImpl::deallocate(void *Ptr, bool &ToPool) { +void DisjointPool::AllocImpl::deallocate(void *Ptr, bool &ToPool) try { auto *SlabPtr = AlignPtrDown(Ptr, SlabMinSize()); // Lock the map on read @@ -846,6 +862,8 @@ void DisjointPool::AllocImpl::deallocate(void *Ptr, bool &ToPool) { // to some slab with an entry in the map. So we find a slab // but the range checks fail. memoryProviderFree(getMemHandle(), Ptr); +} catch (MemoryProviderError &e) { + uma::getPoolLastStatusRef() = e.code; } void DisjointPool::AllocImpl::printStats(bool &TitlePrinted, @@ -938,7 +956,7 @@ void DisjointPool::free(void *ptr) { } enum uma_result_t DisjointPool::get_last_allocation_error() { - return UMA_RESULT_ERROR_UNKNOWN; + return uma::getPoolLastStatusRef(); } DisjointPool::DisjointPool() {} From 599942ae9d6def24d49b4735c4e24c9e2c7e45c0 Mon Sep 17 00:00:00 2001 From: Igor Chorazewicz Date: Tue, 20 Jun 2023 19:09:09 +0000 Subject: [PATCH 3/4] [uma] simplify get_name by returning the name directly instead of through out param. This function is not allowed to fail anyway. --- source/common/uma_helpers.hpp | 2 +- .../include/uma/memory_provider.h | 3 +-- .../include/uma/memory_provider_ops.h | 2 +- .../unified_memory_allocation/src/memory_provider.c | 5 ++--- .../unified_memory_allocation/src/memory_tracker.cpp | 4 ++-- test/unified_memory_allocation/common/provider.c | 8 ++++---- test/unified_memory_allocation/common/provider.hpp | 4 ++-- test/unified_memory_allocation/memoryProviderAPI.cpp | 3 +-- 8 files changed, 14 insertions(+), 17 deletions(-) diff --git a/source/common/uma_helpers.hpp b/source/common/uma_helpers.hpp index 2d7d9a4059..95a9c04f05 100644 --- a/source/common/uma_helpers.hpp +++ b/source/common/uma_helpers.hpp @@ -92,7 +92,7 @@ auto memoryProviderMakeUnique(Args &&...args) { UMA_ASSIGN_OP(ops, T, get_min_page_size, UMA_RESULT_ERROR_UNKNOWN); UMA_ASSIGN_OP(ops, T, purge_lazy, UMA_RESULT_ERROR_UNKNOWN); UMA_ASSIGN_OP(ops, T, purge_force, UMA_RESULT_ERROR_UNKNOWN); - UMA_ASSIGN_OP_NORETURN(ops, T, get_name); + UMA_ASSIGN_OP(ops, T, get_name, ""); uma_memory_provider_handle_t hProvider = nullptr; auto ret = umaMemoryProviderCreate(&ops, &argsTuple, &hProvider); diff --git a/source/common/unified_memory_allocation/include/uma/memory_provider.h b/source/common/unified_memory_allocation/include/uma/memory_provider.h index 8201d54f22..cb0f62ccb5 100644 --- a/source/common/unified_memory_allocation/include/uma/memory_provider.h +++ b/source/common/unified_memory_allocation/include/uma/memory_provider.h @@ -138,8 +138,7 @@ umaMemoryProviderPurgeForce(uma_memory_provider_handle_t hProvider, void *ptr, /// \brief Retrive name of a given memory provider. /// \param hProvider handle to the memory provider /// \param ppName [out] pointer to a string containing name of the provider -void umaMemoryProviderGetName(uma_memory_provider_handle_t hProvider, - const char **ppName); +const char *umaMemoryProviderGetName(uma_memory_provider_handle_t hProvider); #ifdef __cplusplus } diff --git a/source/common/unified_memory_allocation/include/uma/memory_provider_ops.h b/source/common/unified_memory_allocation/include/uma/memory_provider_ops.h index e5afbdaa53..404f221cb3 100644 --- a/source/common/unified_memory_allocation/include/uma/memory_provider_ops.h +++ b/source/common/unified_memory_allocation/include/uma/memory_provider_ops.h @@ -49,7 +49,7 @@ struct uma_memory_provider_ops_t { size_t *pageSize); enum uma_result_t (*purge_lazy)(void *provider, void *ptr, size_t size); enum uma_result_t (*purge_force)(void *provider, void *ptr, size_t size); - void (*get_name)(void *provider, const char **ppName); + const char *(*get_name)(void *provider); }; #ifdef __cplusplus diff --git a/source/common/unified_memory_allocation/src/memory_provider.c b/source/common/unified_memory_allocation/src/memory_provider.c index 345bc8e6ab..5ce2bbadbd 100644 --- a/source/common/unified_memory_allocation/src/memory_provider.c +++ b/source/common/unified_memory_allocation/src/memory_provider.c @@ -99,7 +99,6 @@ umaMemoryProviderPurgeForce(uma_memory_provider_handle_t hProvider, void *ptr, return hProvider->ops.purge_force(hProvider->provider_priv, ptr, size); } -void umaMemoryProviderGetName(uma_memory_provider_handle_t hProvider, - const char **ppName) { - hProvider->ops.get_name(hProvider->provider_priv, ppName); +const char *umaMemoryProviderGetName(uma_memory_provider_handle_t hProvider) { + return hProvider->ops.get_name(hProvider->provider_priv); } diff --git a/source/common/unified_memory_allocation/src/memory_tracker.cpp b/source/common/unified_memory_allocation/src/memory_tracker.cpp index 0dc118b738..32f7dff0b1 100644 --- a/source/common/unified_memory_allocation/src/memory_tracker.cpp +++ b/source/common/unified_memory_allocation/src/memory_tracker.cpp @@ -204,10 +204,10 @@ static enum uma_result_t trackingPurgeForce(void *provider, void *ptr, return umaMemoryProviderPurgeForce(p->hUpstream, ptr, size); } -static void trackingName(void *provider, const char **ppName) { +static const char *trackingName(void *provider) { uma_tracking_memory_provider_t *p = (uma_tracking_memory_provider_t *)provider; - return umaMemoryProviderGetName(p->hUpstream, ppName); + return umaMemoryProviderGetName(p->hUpstream); } enum uma_result_t umaTrackingMemoryProviderCreate( diff --git a/test/unified_memory_allocation/common/provider.c b/test/unified_memory_allocation/common/provider.c index 55968223da..a180705a58 100644 --- a/test/unified_memory_allocation/common/provider.c +++ b/test/unified_memory_allocation/common/provider.c @@ -73,9 +73,9 @@ static enum uma_result_t nullPurgeForce(void *provider, void *ptr, return UMA_RESULT_SUCCESS; } -static void nullName(void *provider, const char **ppName) { +static const char *nullName(void *provider) { (void)provider; - *ppName = "null"; + return "null"; } uma_memory_provider_handle_t nullProviderCreate(void) { @@ -178,11 +178,11 @@ static enum uma_result_t tracePurgeForce(void *provider, void *ptr, size); } -static void traceName(void *provider, const char **ppName) { +static const char *traceName(void *provider) { struct traceParams *traceProvider = (struct traceParams *)provider; traceProvider->trace("name"); - umaMemoryProviderGetName(traceProvider->hUpstreamProvider, ppName); + return umaMemoryProviderGetName(traceProvider->hUpstreamProvider); } uma_memory_provider_handle_t diff --git a/test/unified_memory_allocation/common/provider.hpp b/test/unified_memory_allocation/common/provider.hpp index 6030990f52..bca4f66872 100644 --- a/test/unified_memory_allocation/common/provider.hpp +++ b/test/unified_memory_allocation/common/provider.hpp @@ -47,7 +47,7 @@ struct provider_base { enum uma_result_t purge_force(void *ptr, size_t size) noexcept { return UMA_RESULT_ERROR_UNKNOWN; } - void get_name(const char **ppName) noexcept { *ppName = "base"; } + const char *get_name() noexcept { return "base"; } }; struct provider_malloc : public provider_base { @@ -73,7 +73,7 @@ struct provider_malloc : public provider_base { #endif return UMA_RESULT_SUCCESS; } - void get_name(const char **ppName) noexcept { *ppName = "malloc"; } + const char *get_name() noexcept { return "malloc"; } }; } // namespace uma_test diff --git a/test/unified_memory_allocation/memoryProviderAPI.cpp b/test/unified_memory_allocation/memoryProviderAPI.cpp index 510c2b88be..754b86992e 100644 --- a/test/unified_memory_allocation/memoryProviderAPI.cpp +++ b/test/unified_memory_allocation/memoryProviderAPI.cpp @@ -60,8 +60,7 @@ TEST_F(test, memoryProviderTrace) { ASSERT_EQ(calls["purge_force"], 1); ASSERT_EQ(calls.size(), ++call_count); - const char *pName; - umaMemoryProviderGetName(tracingProvider.get(), &pName); + const char *pName = umaMemoryProviderGetName(tracingProvider.get()); ASSERT_EQ(calls["name"], 1); ASSERT_EQ(calls.size(), ++call_count); ASSERT_EQ(std::string(pName), std::string("null")); From 6f0df6c6ca6bf7e963145af47f2eb486db7bb69d Mon Sep 17 00:00:00 2001 From: Igor Chorazewicz Date: Wed, 28 Jun 2023 18:19:17 +0000 Subject: [PATCH 4/4] [uma] add umaGetLastFailedMemoryProvider which returns handle to the last failed provider. This makes error reporting API independent from how memory pool is using the providers (i.e. whether there is only a single provider or multiple) --- .../unified_memory_allocation/CMakeLists.txt | 1 + .../include/uma/memory_provider.h | 10 +++ .../src/memory_provider.c | 44 ++++++++-- .../src/memory_provider_get_last_failed.cpp | 20 +++++ .../src/memory_provider_internal.h | 1 + .../unified_memory_allocation/common/pool.hpp | 8 +- .../memoryPoolAPI.cpp | 87 +++++++++++++++++++ 7 files changed, 161 insertions(+), 10 deletions(-) create mode 100644 source/common/unified_memory_allocation/src/memory_provider_get_last_failed.cpp diff --git a/source/common/unified_memory_allocation/CMakeLists.txt b/source/common/unified_memory_allocation/CMakeLists.txt index 81eb655692..7be1ab2dce 100644 --- a/source/common/unified_memory_allocation/CMakeLists.txt +++ b/source/common/unified_memory_allocation/CMakeLists.txt @@ -7,6 +7,7 @@ set(UMA_SOURCES src/memory_pool.c src/memory_provider.c src/memory_tracker.cpp + src/memory_provider_get_last_failed.cpp ) if(UMA_BUILD_SHARED_LIBRARY) diff --git a/source/common/unified_memory_allocation/include/uma/memory_provider.h b/source/common/unified_memory_allocation/include/uma/memory_provider.h index cb0f62ccb5..9b65dc6419 100644 --- a/source/common/unified_memory_allocation/include/uma/memory_provider.h +++ b/source/common/unified_memory_allocation/include/uma/memory_provider.h @@ -140,6 +140,16 @@ umaMemoryProviderPurgeForce(uma_memory_provider_handle_t hProvider, void *ptr, /// \param ppName [out] pointer to a string containing name of the provider const char *umaMemoryProviderGetName(uma_memory_provider_handle_t hProvider); +/// \brief Retrieve handle to the last memory provider that returned status other +/// than UMA_RESULT_SUCCESS on the calling thread. +/// +/// \details Handle to the memory provider is stored in the thread local +/// storage. The handle is updated every time a memory provider +/// returns status other than UMA_RESULT_SUCCESS. +/// +/// \return Handle to the memory provider +uma_memory_provider_handle_t umaGetLastFailedMemoryProvider(); + #ifdef __cplusplus } #endif diff --git a/source/common/unified_memory_allocation/src/memory_provider.c b/source/common/unified_memory_allocation/src/memory_provider.c index 5ce2bbadbd..103bcef88a 100644 --- a/source/common/unified_memory_allocation/src/memory_provider.c +++ b/source/common/unified_memory_allocation/src/memory_provider.c @@ -51,15 +51,29 @@ void umaMemoryProviderDestroy(uma_memory_provider_handle_t hProvider) { free(hProvider); } +static void +checkErrorAndSetLastProvider(enum uma_result_t result, + uma_memory_provider_handle_t hProvider) { + if (result != UMA_RESULT_SUCCESS) { + *umaGetLastFailedMemoryProviderPtr() = hProvider; + } +} + enum uma_result_t umaMemoryProviderAlloc(uma_memory_provider_handle_t hProvider, size_t size, size_t alignment, void **ptr) { - return hProvider->ops.alloc(hProvider->provider_priv, size, alignment, ptr); + enum uma_result_t res = + hProvider->ops.alloc(hProvider->provider_priv, size, alignment, ptr); + checkErrorAndSetLastProvider(res, hProvider); + return res; } enum uma_result_t umaMemoryProviderFree(uma_memory_provider_handle_t hProvider, void *ptr, size_t size) { - return hProvider->ops.free(hProvider->provider_priv, ptr, size); + enum uma_result_t res = + hProvider->ops.free(hProvider->provider_priv, ptr, size); + checkErrorAndSetLastProvider(res, hProvider); + return res; } void umaMemoryProviderGetLastNativeError(uma_memory_provider_handle_t hProvider, @@ -76,29 +90,43 @@ void *umaMemoryProviderGetPriv(uma_memory_provider_handle_t hProvider) { enum uma_result_t umaMemoryProviderGetRecommendedPageSize(uma_memory_provider_handle_t hProvider, size_t size, size_t *pageSize) { - return hProvider->ops.get_recommended_page_size(hProvider->provider_priv, - size, pageSize); + enum uma_result_t res = hProvider->ops.get_recommended_page_size( + hProvider->provider_priv, size, pageSize); + checkErrorAndSetLastProvider(res, hProvider); + return res; } enum uma_result_t umaMemoryProviderGetMinPageSize(uma_memory_provider_handle_t hProvider, void *ptr, size_t *pageSize) { - return hProvider->ops.get_min_page_size(hProvider->provider_priv, ptr, - pageSize); + enum uma_result_t res = hProvider->ops.get_min_page_size( + hProvider->provider_priv, ptr, pageSize); + checkErrorAndSetLastProvider(res, hProvider); + return res; } enum uma_result_t umaMemoryProviderPurgeLazy(uma_memory_provider_handle_t hProvider, void *ptr, size_t size) { - return hProvider->ops.purge_lazy(hProvider->provider_priv, ptr, size); + enum uma_result_t res = + hProvider->ops.purge_lazy(hProvider->provider_priv, ptr, size); + checkErrorAndSetLastProvider(res, hProvider); + return res; } enum uma_result_t umaMemoryProviderPurgeForce(uma_memory_provider_handle_t hProvider, void *ptr, size_t size) { - return hProvider->ops.purge_force(hProvider->provider_priv, ptr, size); + enum uma_result_t res = + hProvider->ops.purge_force(hProvider->provider_priv, ptr, size); + checkErrorAndSetLastProvider(res, hProvider); + return res; } const char *umaMemoryProviderGetName(uma_memory_provider_handle_t hProvider) { return hProvider->ops.get_name(hProvider->provider_priv); } + +uma_memory_provider_handle_t umaGetLastFailedMemoryProvider() { + return *umaGetLastFailedMemoryProviderPtr(); +} diff --git a/source/common/unified_memory_allocation/src/memory_provider_get_last_failed.cpp b/source/common/unified_memory_allocation/src/memory_provider_get_last_failed.cpp new file mode 100644 index 0000000000..52a4e6ce35 --- /dev/null +++ b/source/common/unified_memory_allocation/src/memory_provider_get_last_failed.cpp @@ -0,0 +1,20 @@ +/* + * + * 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_provider_internal.h" + +extern "C" { + +static thread_local uma_memory_provider_handle_t lastFailedProvider = nullptr; + +uma_memory_provider_handle_t *umaGetLastFailedMemoryProviderPtr() { + return &lastFailedProvider; +} +} diff --git a/source/common/unified_memory_allocation/src/memory_provider_internal.h b/source/common/unified_memory_allocation/src/memory_provider_internal.h index dce77cb6d6..a2b190488f 100644 --- a/source/common/unified_memory_allocation/src/memory_provider_internal.h +++ b/source/common/unified_memory_allocation/src/memory_provider_internal.h @@ -18,6 +18,7 @@ extern "C" { #endif void *umaMemoryProviderGetPriv(uma_memory_provider_handle_t hProvider); +uma_memory_provider_handle_t *umaGetLastFailedMemoryProviderPtr(); #ifdef __cplusplus } diff --git a/test/unified_memory_allocation/common/pool.hpp b/test/unified_memory_allocation/common/pool.hpp index f9830f5f83..6e1552140d 100644 --- a/test/unified_memory_allocation/common/pool.hpp +++ b/test/unified_memory_allocation/common/pool.hpp @@ -81,7 +81,9 @@ struct proxy_pool : public pool_base { memset(ptr, 0, num * size); - EXPECT_EQ_NOEXCEPT(ret, UMA_RESULT_SUCCESS); + if (ptr) { + EXPECT_EQ_NOEXCEPT(ret, UMA_RESULT_SUCCESS); + } return ptr; } void *realloc(void *ptr, size_t size) noexcept { @@ -91,7 +93,9 @@ struct proxy_pool : public pool_base { void *aligned_malloc(size_t size, size_t alignment) noexcept { void *ptr; auto ret = umaMemoryProviderAlloc(provider, size, alignment, &ptr); - EXPECT_EQ_NOEXCEPT(ret, UMA_RESULT_SUCCESS); + if (ptr) { + EXPECT_EQ_NOEXCEPT(ret, UMA_RESULT_SUCCESS); + } return ptr; } size_t malloc_usable_size(void *ptr) noexcept { diff --git a/test/unified_memory_allocation/memoryPoolAPI.cpp b/test/unified_memory_allocation/memoryPoolAPI.cpp index ffcc3afed8..015d3e1dd5 100644 --- a/test/unified_memory_allocation/memoryPoolAPI.cpp +++ b/test/unified_memory_allocation/memoryPoolAPI.cpp @@ -14,6 +14,7 @@ #include #include +#include #include using uma_test::test; @@ -222,3 +223,89 @@ TEST_F(test, retrieveMemoryProvidersError) { ret = umaPoolGetMemoryProviders(pool.get(), 1, providers.data(), nullptr); ASSERT_EQ(ret, UMA_RESULT_ERROR_INVALID_ARGUMENT); } + +// TODO: extend test for different functions (not only alloc) +TEST_F(test, getLastFailedMemoryProvider) { + static constexpr size_t allocSize = 8; + static uma_result_t allocResult = UMA_RESULT_SUCCESS; + + struct memory_provider : public uma_test::provider_base { + uma_result_t initialize(const char *name) { + this->name = name; + return UMA_RESULT_SUCCESS; + } + + enum uma_result_t alloc(size_t size, size_t, void **ptr) noexcept { + if (allocResult == UMA_RESULT_SUCCESS) { + *ptr = malloc(size); + } else { + *ptr = nullptr; + } + + return allocResult; + } + + enum uma_result_t free(void *ptr, size_t size) noexcept { + ::free(ptr); + return UMA_RESULT_SUCCESS; + } + + const char *get_name() noexcept { return this->name; } + + const char *name; + }; + + auto [ret1, providerUnique1] = + uma::memoryProviderMakeUnique("provider1"); + ASSERT_EQ(ret1, UMA_RESULT_SUCCESS); + auto [ret2, providerUnique2] = + uma::memoryProviderMakeUnique("provider2"); + ASSERT_EQ(ret2, UMA_RESULT_SUCCESS); + + auto hProvider = providerUnique1.get(); + + auto [ret, pool] = uma::poolMakeUnique(&hProvider, 1); + ASSERT_EQ(ret, UMA_RESULT_SUCCESS); + + ASSERT_EQ(umaGetLastFailedMemoryProvider(), nullptr); + auto ptr = umaPoolMalloc(pool.get(), allocSize); + ASSERT_NE(ptr, nullptr); + ASSERT_EQ(umaGetLastFailedMemoryProvider(), nullptr); + + // make provider return an error during allocation + allocResult = UMA_RESULT_ERROR_UNKNOWN; + ptr = umaPoolMalloc(pool.get(), allocSize); + ASSERT_EQ(ptr, nullptr); + ASSERT_EQ(std::string_view( + umaMemoryProviderGetName(umaGetLastFailedMemoryProvider())), + "provider1"); + + ret = umaMemoryProviderAlloc(providerUnique2.get(), allocSize, 0, &ptr); + ASSERT_EQ(ptr, nullptr); + ASSERT_EQ(std::string_view( + umaMemoryProviderGetName(umaGetLastFailedMemoryProvider())), + "provider2"); + + // succesfull provider should not be returned by umaGetLastFailedMemoryProvider + allocResult = UMA_RESULT_SUCCESS; + ptr = umaPoolMalloc(pool.get(), allocSize); + ASSERT_NE(ptr, nullptr); + ASSERT_EQ(std::string_view( + umaMemoryProviderGetName(umaGetLastFailedMemoryProvider())), + "provider2"); + + // erorr in another thread should not impact umaGetLastFailedMemoryProvider on this thread + allocResult = UMA_RESULT_ERROR_UNKNOWN; + std::thread t([&, hPool = pool.get()] { + ptr = umaPoolMalloc(hPool, allocSize); + ASSERT_EQ(ptr, nullptr); + ASSERT_EQ(std::string_view(umaMemoryProviderGetName( + umaGetLastFailedMemoryProvider())), + "provider1"); + }); + t.join(); + + ASSERT_EQ(std::string_view( + umaMemoryProviderGetName(umaGetLastFailedMemoryProvider())), + "provider2"); +}