Skip to content

Allow to create a pool with NULL params #1409

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion include/umf/memory_pool.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ typedef uint32_t umf_pool_create_flags_t;
/// @brief Creates new memory pool.
/// @param ops instance of umf_memory_pool_ops_t
/// @param provider memory provider that will be used for coarse-grain allocations.
/// @param params pointer to pool-specific parameters
/// @param params pointer to pool-specific parameters, or NULL for defaults
/// @param flags a combination of umf_pool_create_flag_t
/// @param hPool [out] handle to the newly created memory pool
/// @return UMF_RESULT_SUCCESS on success or appropriate error code on failure.
Expand Down
2 changes: 1 addition & 1 deletion include/umf/memory_pool_ops.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ typedef struct umf_memory_pool_ops_t {
/// @param providers array of memory providers that will be used for coarse-grain allocations.
/// Should contain at least one memory provider.
/// @param numProvider number of elements in the providers array
/// @param params pool-specific params
/// @param params pool-specific params, or NULL for defaults
/// @param pool [out] returns pointer to the pool
/// @return UMF_RESULT_SUCCESS on success or appropriate error code on failure.
///
Expand Down
38 changes: 18 additions & 20 deletions src/pool/pool_disjoint.c
Original file line number Diff line number Diff line change
Expand Up @@ -644,20 +644,29 @@ static void free_slab(void *unused, void *slab) {
}
}

static const umf_disjoint_pool_params_t default_params = {
.slab_min_size = 64 * 1024, // 64KB default
.max_poolable_size = 2 * 1024 * 1024, // 2MB default
.capacity = 4, // default
.min_bucket_size = 8, // default
.cur_pool_size = 0,
.pool_trace = 0,
.shared_limits = NULL,
.name = "disjoint"};

umf_result_t disjoint_pool_initialize(umf_memory_provider_handle_t provider,
const void *params, void **ppPool) {
// TODO set defaults when user pass the NULL as params
if (!provider || !params || !ppPool) {
if (!provider || !ppPool) {
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
}

const umf_disjoint_pool_params_t *dp_params = params;
const umf_disjoint_pool_params_t *dp_params;

// min_bucket_size parameter must be a power of 2 for bucket sizes
// to generate correctly.
if (!dp_params->min_bucket_size ||
!IS_POWER_OF_2(dp_params->min_bucket_size)) {
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
// If params is NULL, use default values
if (!params) {
dp_params = &default_params;
} else {
dp_params = params;
}

disjoint_pool_t *disjoint_pool =
Expand Down Expand Up @@ -1102,18 +1111,7 @@ umfDisjointPoolParamsCreate(umf_disjoint_pool_params_handle_t *hParams) {
return UMF_RESULT_ERROR_OUT_OF_HOST_MEMORY;
}

*params = (umf_disjoint_pool_params_t){
.slab_min_size = 64 * 1024, // 64K
.max_poolable_size = 2 * 1024 * 1024, // 2MB
.capacity = 4,
.min_bucket_size = UMF_DISJOINT_POOL_MIN_BUCKET_DEFAULT_SIZE,
.cur_pool_size = 0,
.pool_trace = 0,
.shared_limits = NULL,
};

strncpy(params->name, DEFAULT_NAME, sizeof(params->name) - 1);
params->name[sizeof(params->name) - 1] = '\0';
*params = default_params;

*hParams = params;
return UMF_RESULT_SUCCESS;
Expand Down
16 changes: 10 additions & 6 deletions src/pool/pool_jemalloc.c
Original file line number Diff line number Diff line change
Expand Up @@ -431,15 +431,19 @@ static umf_result_t op_initialize(umf_memory_provider_handle_t provider,

extent_hooks_t *pHooks = &arena_extent_hooks;
size_t unsigned_size = sizeof(unsigned);
int n_arenas_set_from_params = 0;
int err;
const umf_jemalloc_pool_params_t *jemalloc_params = params;
const umf_jemalloc_pool_params_t *jemalloc_params;

size_t n_arenas = 0;
if (jemalloc_params) {
n_arenas = jemalloc_params->n_arenas;
n_arenas_set_from_params = 1;
// If params is NULL, use default values
umf_jemalloc_pool_params_t default_params = {}; // Will be calculated later

if (!params) {
jemalloc_params = &default_params;
} else {
jemalloc_params = params;
}
size_t n_arenas = jemalloc_params->n_arenas;
int n_arenas_set_from_params = (params != NULL);

if (n_arenas == 0) {
n_arenas = utils_get_num_cores() * 4;
Expand Down
1 change: 1 addition & 0 deletions src/pool/pool_scalable.c
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,7 @@ static umf_result_t tbb_pool_initialize(umf_memory_provider_handle_t provider,
.keep_all_memory = false,
.reserved = 0};

// If params is provided, override defaults
if (params) {
const umf_scalable_pool_params_t *scalable_params = params;
policy.granularity = scalable_params->granularity;
Expand Down
5 changes: 5 additions & 0 deletions test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,11 @@ add_umf_test(
${BA_SOURCES_FOR_TEST}
LIBS ${UMF_UTILS_FOR_TEST})

add_umf_test(
NAME pool_null_params
SRCS test_pool_null_params.cpp
LIBS ${UMF_UTILS_FOR_TEST})

add_umf_test(
NAME c_api_disjoint_pool
SRCS c_api/disjoint_pool.c
Expand Down
61 changes: 61 additions & 0 deletions test/test_pool_null_params.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/*
* Copyright (C) 2025 Intel Corporation
*
* Under the Apache License v2.0 with LLVM Exceptions. See LICENSE.TXT.
* SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
*/

#include <gtest/gtest.h>

#include <umf/memory_pool.h>
#include <umf/memory_provider.h>
#include <umf/pools/pool_disjoint.h>
#include <umf/pools/pool_jemalloc.h>
#include <umf/pools/pool_proxy.h>
#include <umf/pools/pool_scalable.h>

#include "provider_null.h"

// Dummy provider implementation for testing
static umf_memory_provider_ops_t dummy_provider_ops = UMF_NULL_PROVIDER_OPS;

using PoolOpsFn = const umf_memory_pool_ops_t *(*)();

class PoolNullParamsTest : public ::testing::TestWithParam<PoolOpsFn> {
protected:
umf_memory_provider_handle_t provider = NULL;
void SetUp() override {
ASSERT_EQ(umfMemoryProviderCreate(&dummy_provider_ops, NULL, &provider),
UMF_RESULT_SUCCESS);
}
void TearDown() override {
if (provider) {
umfMemoryProviderDestroy(provider);
}
}
};

TEST_P(PoolNullParamsTest, CreateWithNullParams) {
umf_memory_pool_handle_t pool;
PoolOpsFn opsFn = GetParam();
umf_result_t res = umfPoolCreate(opsFn(), provider, NULL, 0, &pool);
ASSERT_EQ(res, UMF_RESULT_SUCCESS);
umfPoolDestroy(pool);
}

namespace {
const PoolOpsFn poolOpsList[] = {
#if defined(UMF_POOL_SCALABLE_ENABLED)
&umfScalablePoolOps,
#endif
#if defined(UMF_POOL_JEMALLOC_ENABLED)
&umfJemallocPoolOps,
#endif
#if defined(UMF_POOL_PROXY_ENABLED)
&umfProxyPoolOps
#endif
&umfDisjointPoolOps};
} // namespace

INSTANTIATE_TEST_SUITE_P(poolNullParamsTest, PoolNullParamsTest,
::testing::ValuesIn(poolOpsList));