Skip to content

Commit e296e31

Browse files
lukaszstolarczuklukaszstolarczuk
authored
Merge pull request #1465 from ldorau/Verify_if_the_provider_supports_the_split_operation
Verify if the provider supports the split operation
2 parents 1de269c + 73858f2 commit e296e31

File tree

4 files changed

+119
-30
lines changed

4 files changed

+119
-30
lines changed

‎.github/workflows/reusable_compatibility.yml‎

Lines changed: 38 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -95,10 +95,21 @@ jobs:
9595
9696
- name: Run "tag" UMF tests with latest UMF libs (warnings enabled)
9797
working-directory: ${{github.workspace}}/tag_version/build
98+
# Exclude the test_jemalloc_pool test -
99+
# TODO: add fix for that in v1.0.1
98100
run: >
99101
UMF_LOG="level:warning;flush:debug;output:stderr;pid:no"
100102
LD_LIBRARY_PATH=${{github.workspace}}/latest_version/build/lib/
101-
ctest --verbose
103+
ctest --verbose -E test_jemalloc_pool
104+
105+
- name: Run EXCLUDED tests with filters
106+
working-directory: ${{github.workspace}}/tag_version/build
107+
# Exclude the jemallocPoolName test case of the test_jemalloc_pool test
108+
# TODO: add fix for that in v1.0.1
109+
run: >
110+
UMF_LOG="level:warning;flush:debug;output:stderr;pid:no"
111+
LD_LIBRARY_PATH=${{github.workspace}}/latest_version/build/lib/
112+
./test/test_jemalloc_pool --gtest_filter="-*jemallocPoolName*"
102113
103114
# Browse all folders in the examples directory, build them using the
104115
# latest UMF version, and run them, excluding those in the exclude list.
@@ -220,10 +231,22 @@ jobs:
220231

221232
- name: Run "tag" UMF tests with latest UMF libs (warnings enabled)
222233
working-directory: ${{github.workspace}}/tag_version/build
234+
# Exclude the test_jemalloc_pool test -
235+
# TODO: add fix for that in v1.0.1
223236
run: |
224-
$env:UMF_LOG="level:warning;flush:debug;output:stderr;pid:no"
237+
$env:UMF_LOG="level:warning;flush:debug;output:stderr;pid:no"
225238
cp ${{github.workspace}}/latest_version/build/bin/Debug/umf.dll ${{github.workspace}}/tag_version/build/bin/Debug/umf.dll
226-
ctest -C Debug --verbose
239+
ctest -C Debug --verbose -E test_jemalloc_pool
240+
241+
- name: Run EXCLUDED tests with filters
242+
working-directory: ${{github.workspace}}/tag_version/build/
243+
# Exclude the jemallocPoolName test case of the test_jemalloc_pool test
244+
# TODO: add fix for that in v1.0.1
245+
run: |
246+
$env:UMF_LOG="level:warning;flush:debug;output:stderr;pid:no"
247+
$env:Path = "${{github.workspace}}/tag_version/build/bin/Debug;${{env.VCPKG_BIN_PATH}};$env:Path"
248+
cp ${{github.workspace}}/latest_version/build/bin/Debug/umf.dll ${{github.workspace}}/tag_version/build/bin/Debug/umf.dll
249+
test/Debug/test_jemalloc_pool.exe --gtest_filter="-*jemallocPoolName*"
227250
228251
# Browse all folders in the examples directory, build them using the
229252
# latest UMF version, and run them, excluding those in the exclude list.
@@ -361,10 +384,21 @@ jobs:
361384
362385
- name: Run "tag" UMF tests with latest UMF libs (warnings enabled)
363386
working-directory: ${{github.workspace}}/tag_version/build
387+
# Exclude the test_jemalloc_pool test -
388+
# TODO: add fix for that in v1.0.1
389+
run: >
390+
UMF_LOG="level:warning;flush:debug;output:stderr;pid:no"
391+
LD_LIBRARY_PATH=${{github.workspace}}/latest_version/build/lib/
392+
ctest --verbose -E test_jemalloc_pool
393+
394+
- name: Run EXCLUDED tests with filters
395+
working-directory: ${{github.workspace}}/tag_version/build
396+
# Exclude the jemallocPoolName test case of the test_jemalloc_pool test
397+
# TODO: add fix for that in v1.0.1
364398
run: >
365399
UMF_LOG="level:warning;flush:debug;output:stderr;pid:no"
366400
LD_LIBRARY_PATH=${{github.workspace}}/latest_version/build/lib/
367-
ctest --verbose
401+
./test/test_jemalloc_pool --gtest_filter="-*jemallocPoolName*"
368402
369403
# Browse all folders in the examples directory, build them using the
370404
# latest UMF version, and run them, excluding those in the exclude list.

‎src/pool/pool_jemalloc.c‎

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
#include <string.h>
1313

1414
#include "base_alloc_global.h"
15+
#include "memory_provider_internal.h"
16+
#include "provider_tracking.h"
1517
#include "utils_common.h"
1618
#include "utils_concurrency.h"
1719
#include "utils_log.h"
@@ -272,8 +274,15 @@ static bool arena_extent_split(extent_hooks_t *extent_hooks, void *addr,
272274

273275
jemalloc_memory_pool_t *pool = get_pool_by_arena_index(arena_ind);
274276
assert(pool);
275-
return umfMemoryProviderAllocationSplit(pool->provider, addr, size,
276-
size_a) != UMF_RESULT_SUCCESS;
277+
278+
umf_result_t ret =
279+
umfMemoryProviderAllocationSplit(pool->provider, addr, size, size_a);
280+
if (ret != UMF_RESULT_SUCCESS) {
281+
LOG_ERR("memory provider failed to split a memory region, while "
282+
"jemalloc requires that");
283+
}
284+
285+
return ret != UMF_RESULT_SUCCESS;
277286
}
278287

279288
// arena_extent_merge - an extent merge function conforms to the extent_merge_t type and optionally
@@ -424,11 +433,45 @@ static void *op_aligned_alloc(void *pool, size_t size, size_t alignment) {
424433
return ptr;
425434
}
426435

436+
// Verify if the memory provider supports the split() operation,
437+
// because jemalloc pool requires that.
438+
static umf_result_t verify_split(umf_memory_provider_handle_t provider) {
439+
// Retrieve the upstream memory provider
440+
umf_memory_provider_handle_t upstream_provider = NULL;
441+
umfTrackingMemoryProviderGetUpstreamProvider(
442+
umfMemoryProviderGetPriv(provider), &upstream_provider);
443+
444+
size_t page_size = 0;
445+
umf_result_t ret =
446+
umfMemoryProviderGetMinPageSize(upstream_provider, NULL, &page_size);
447+
if (ret != UMF_RESULT_SUCCESS) {
448+
return ret;
449+
}
450+
451+
size_t size = 2 * page_size; // use double the page size for the split test
452+
if (UMF_RESULT_ERROR_NOT_SUPPORTED ==
453+
umfMemoryProviderAllocationSplit(upstream_provider, (void *)size, size,
454+
page_size)) {
455+
LOG_ERR("memory provider does not support the split operation, while "
456+
"jemalloc pool requires that");
457+
return UMF_RESULT_ERROR_NOT_SUPPORTED;
458+
}
459+
460+
return UMF_RESULT_SUCCESS;
461+
}
462+
427463
static umf_result_t op_initialize(umf_memory_provider_handle_t provider,
428464
const void *params, void **out_pool) {
429465
assert(provider);
430466
assert(out_pool);
431467

468+
// Verify if the memory provider supports the split() operation,
469+
// because jemalloc pool requires that.
470+
umf_result_t ret = verify_split(provider);
471+
if (ret != UMF_RESULT_SUCCESS) {
472+
return ret;
473+
}
474+
432475
extent_hooks_t *pHooks = &arena_extent_hooks;
433476
size_t unsigned_size = sizeof(unsigned);
434477
int n_arenas_set_from_params = 0;

‎test/common/provider.hpp‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -36,69 +36,69 @@ auto wrapProviderUnique(umf_memory_provider_handle_t hProvider) {
3636
typedef struct provider_base_t {
3737
umf_result_t initialize() noexcept { return UMF_RESULT_SUCCESS; };
3838
umf_result_t alloc(size_t, size_t, void **) noexcept {
39-
return UMF_RESULT_ERROR_UNKNOWN;
39+
return UMF_RESULT_ERROR_NOT_SUPPORTED;
4040
}
4141
umf_result_t free([[maybe_unused]] void *ptr,
4242
[[maybe_unused]] size_t size) noexcept {
43-
return UMF_RESULT_ERROR_UNKNOWN;
43+
return UMF_RESULT_ERROR_NOT_SUPPORTED;
4444
}
4545
umf_result_t get_last_native_error(const char **, int32_t *) noexcept {
46-
return UMF_RESULT_ERROR_UNKNOWN;
46+
return UMF_RESULT_ERROR_NOT_SUPPORTED;
4747
}
4848
umf_result_t
4949
get_recommended_page_size([[maybe_unused]] size_t size,
5050
[[maybe_unused]] size_t *pageSize) noexcept {
51-
return UMF_RESULT_ERROR_UNKNOWN;
51+
return UMF_RESULT_ERROR_NOT_SUPPORTED;
5252
}
5353
umf_result_t get_min_page_size([[maybe_unused]] const void *ptr,
5454
[[maybe_unused]] size_t *pageSize) noexcept {
55-
return UMF_RESULT_ERROR_UNKNOWN;
55+
return UMF_RESULT_ERROR_NOT_SUPPORTED;
5656
}
5757
umf_result_t get_name(const char **name) noexcept {
5858
*name = "base";
5959
return UMF_RESULT_SUCCESS;
6060
}
6161
umf_result_t ext_purge_lazy([[maybe_unused]] void *ptr,
6262
[[maybe_unused]] size_t size) noexcept {
63-
return UMF_RESULT_ERROR_UNKNOWN;
63+
return UMF_RESULT_ERROR_NOT_SUPPORTED;
6464
}
6565
umf_result_t ext_purge_force([[maybe_unused]] void *ptr,
6666
[[maybe_unused]] size_t size) noexcept {
67-
return UMF_RESULT_ERROR_UNKNOWN;
67+
return UMF_RESULT_ERROR_NOT_SUPPORTED;
6868
}
6969

7070
umf_result_t ext_allocation_merge([[maybe_unused]] void *lowPtr,
7171
[[maybe_unused]] void *highPtr,
7272
[[maybe_unused]] size_t totalSize) {
73-
return UMF_RESULT_ERROR_UNKNOWN;
73+
return UMF_RESULT_ERROR_NOT_SUPPORTED;
7474
}
7575

7676
umf_result_t ext_allocation_split([[maybe_unused]] void *ptr,
7777
[[maybe_unused]] size_t totalSize,
7878
[[maybe_unused]] size_t firstSize) {
79-
return UMF_RESULT_ERROR_UNKNOWN;
79+
return UMF_RESULT_ERROR_NOT_SUPPORTED;
8080
}
8181
umf_result_t
8282
ext_get_ipc_handle_size([[maybe_unused]] size_t *size) noexcept {
83-
return UMF_RESULT_ERROR_UNKNOWN;
83+
return UMF_RESULT_ERROR_NOT_SUPPORTED;
8484
}
8585
umf_result_t
8686
ext_get_ipc_handle([[maybe_unused]] const void *ptr,
8787
[[maybe_unused]] size_t size,
8888
[[maybe_unused]] void *providerIpcData) noexcept {
89-
return UMF_RESULT_ERROR_UNKNOWN;
89+
return UMF_RESULT_ERROR_NOT_SUPPORTED;
9090
}
9191
umf_result_t
9292
ext_put_ipc_handle([[maybe_unused]] void *providerIpcData) noexcept {
93-
return UMF_RESULT_ERROR_UNKNOWN;
93+
return UMF_RESULT_ERROR_NOT_SUPPORTED;
9494
}
9595
umf_result_t ext_open_ipc_handle([[maybe_unused]] void *providerIpcData,
9696
[[maybe_unused]] void **ptr) noexcept {
97-
return UMF_RESULT_ERROR_UNKNOWN;
97+
return UMF_RESULT_ERROR_NOT_SUPPORTED;
9898
}
9999
umf_result_t ext_close_ipc_handle([[maybe_unused]] void *ptr,
100100
[[maybe_unused]] size_t size) noexcept {
101-
return UMF_RESULT_ERROR_UNKNOWN;
101+
return UMF_RESULT_ERROR_NOT_SUPPORTED;
102102
}
103103
virtual ~provider_base_t() = default;
104104
} provider_base_t;

‎test/pools/jemalloc_pool.cpp‎

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -192,24 +192,36 @@ TEST_F(test, jemallocPoolName) {
192192
umf_jemalloc_pool_params_handle_t params = nullptr;
193193
umf_result_t res = umfJemallocPoolParamsCreate(&params);
194194
EXPECT_EQ(res, UMF_RESULT_SUCCESS);
195-
umf_memory_provider_handle_t provider_handle = nullptr;
196195
umf_memory_pool_handle_t pool = NULL;
197196

198-
struct memory_provider : public umf_test::provider_base_t {};
199-
umf_memory_provider_ops_t provider_ops =
200-
umf_test::providerMakeCOps<memory_provider, void>();
201-
auto providerUnique =
202-
wrapProviderUnique(createProviderChecked(&provider_ops, nullptr));
203-
provider_handle = providerUnique.get();
204-
205-
res =
206-
umfPoolCreate(umfJemallocPoolOps(), provider_handle, params, 0, &pool);
197+
auto nullProvider = nullProviderCreate();
198+
res = umfPoolCreate(umfJemallocPoolOps(), nullProvider, params, 0, &pool);
207199
EXPECT_EQ(res, UMF_RESULT_SUCCESS);
208200
const char *name = nullptr;
209201
res = umfPoolGetName(pool, &name);
210202
EXPECT_EQ(res, UMF_RESULT_SUCCESS);
211203
EXPECT_STREQ(name, "jemalloc");
212204

213205
umfPoolDestroy(pool);
206+
umfMemoryProviderDestroy(nullProvider);
207+
umfJemallocPoolParamsDestroy(params);
208+
}
209+
210+
TEST_F(test, jemallocProviderDoesNotSupportSplit) {
211+
umf_jemalloc_pool_params_handle_t params = nullptr;
212+
umf_result_t res = umfJemallocPoolParamsCreate(&params);
213+
EXPECT_EQ(res, UMF_RESULT_SUCCESS);
214+
215+
umf_memory_provider_handle_t ba_provider;
216+
umf_result_t ret =
217+
umfMemoryProviderCreate(&BA_GLOBAL_PROVIDER_OPS, nullptr, &ba_provider);
218+
ASSERT_EQ(ret, UMF_RESULT_SUCCESS);
219+
220+
umf_memory_pool_handle_t pool = nullptr;
221+
res = umfPoolCreate(umfJemallocPoolOps(), ba_provider, params, 0, &pool);
222+
EXPECT_EQ(res, UMF_RESULT_ERROR_NOT_SUPPORTED);
223+
EXPECT_EQ(pool, nullptr);
224+
225+
umfMemoryProviderDestroy(ba_provider);
214226
umfJemallocPoolParamsDestroy(params);
215227
}

0 commit comments

Comments
 (0)