Skip to content

Commit a1ecd13

Browse files
lplewalplewa
committed
use unique_ptr instead of constructor/destructor in benchmarks
On Windows static builds, destructors are invoked after the UMF destructor, causing parameter structures to be unable to be destroyed. By switching to std::unique_ptr, we ensure that parameters are properly cleaned up and the destruction order issue is resolved.
1 parent 9bf1316 commit a1ecd13

File tree

3 files changed

+85
-66
lines changed

3 files changed

+85
-66
lines changed

‎benchmark/benchmark.cpp‎

Lines changed: 57 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -37,20 +37,29 @@ struct glibc_malloc : public allocator_interface {
3737
};
3838

3939
struct os_provider : public provider_interface {
40-
umf_os_memory_provider_params_handle_t params = NULL;
41-
os_provider() {
42-
umfOsMemoryProviderParamsCreate(&params);
43-
return;
44-
}
45-
46-
~os_provider() {
47-
if (params != NULL) {
48-
umfOsMemoryProviderParamsDestroy(params);
40+
provider_interface::params_ptr
41+
getParams(::benchmark::State &state) override {
42+
umf_os_memory_provider_params_handle_t raw_params = nullptr;
43+
umfOsMemoryProviderParamsCreate(&raw_params);
44+
if (!raw_params) {
45+
state.SkipWithError("Failed to create os provider params");
46+
return {nullptr, [](void *) {}};
4947
}
48+
49+
// Use a lambda as the custom deleter
50+
auto deleter = [](void *p) {
51+
if (p) {
52+
auto handle =
53+
static_cast<umf_os_memory_provider_params_handle_t>(p);
54+
umfOsMemoryProviderParamsDestroy(handle);
55+
}
56+
};
57+
58+
return {static_cast<void *>(raw_params), deleter};
5059
}
5160

52-
void *getParams() override { return params; }
53-
umf_memory_provider_ops_t *getOps() override {
61+
umf_memory_provider_ops_t *
62+
getOps([[maybe_unused]] ::benchmark::State &state) override {
5463
return umfOsMemoryProviderOps();
5564
}
5665
static std::string name() { return "os_provider"; }
@@ -62,73 +71,66 @@ struct proxy_pool : public pool_interface<Provider> {
6271
getOps([[maybe_unused]] ::benchmark::State &state) override {
6372
return umfProxyPoolOps();
6473
}
65-
void *getParams([[maybe_unused]] ::benchmark::State &state) override {
66-
return nullptr;
67-
}
74+
6875
static std::string name() { return "proxy_pool<" + Provider::name() + ">"; }
6976
};
7077

7178
#ifdef UMF_POOL_DISJOINT_ENABLED
7279
template <typename Provider>
7380
struct disjoint_pool : public pool_interface<Provider> {
74-
umf_disjoint_pool_params_handle_t disjoint_memory_pool_params;
81+
umf_memory_pool_ops_t *
82+
getOps([[maybe_unused]] ::benchmark::State &state) override {
83+
return umfDisjointPoolOps();
84+
}
7585

76-
disjoint_pool() {
77-
disjoint_memory_pool_params = NULL;
78-
auto ret = umfDisjointPoolParamsCreate(&disjoint_memory_pool_params);
86+
typename pool_interface<Provider>::params_ptr
87+
getParams(::benchmark::State &state) override {
88+
umf_disjoint_pool_params_handle_t raw_params = nullptr;
89+
auto ret = umfDisjointPoolParamsCreate(&raw_params);
7990
if (ret != UMF_RESULT_SUCCESS) {
80-
return;
91+
state.SkipWithError("Failed to create disjoint pool params");
92+
return {nullptr, [](void *) {}};
8193
}
8294

83-
// those function should never fail, so error handling is minimal.
84-
ret = umfDisjointPoolParamsSetSlabMinSize(disjoint_memory_pool_params,
85-
4096);
95+
ret = umfDisjointPoolParamsSetSlabMinSize(raw_params, 4096);
8696
if (ret != UMF_RESULT_SUCCESS) {
87-
goto err;
97+
state.SkipWithError("Failed to set slab min size");
98+
umfDisjointPoolParamsDestroy(raw_params);
99+
return {nullptr, [](void *) {}};
88100
}
89101

90-
ret = umfDisjointPoolParamsSetCapacity(disjoint_memory_pool_params, 4);
102+
ret = umfDisjointPoolParamsSetCapacity(raw_params, 4);
91103
if (ret != UMF_RESULT_SUCCESS) {
92-
goto err;
104+
state.SkipWithError("Failed to set capacity");
105+
umfDisjointPoolParamsDestroy(raw_params);
106+
return {nullptr, [](void *) {}};
93107
}
94108

95-
ret = umfDisjointPoolParamsSetMinBucketSize(disjoint_memory_pool_params,
96-
4096);
109+
ret = umfDisjointPoolParamsSetMinBucketSize(raw_params, 4096);
97110
if (ret != UMF_RESULT_SUCCESS) {
98-
goto err;
111+
state.SkipWithError("Failed to set min bucket size");
112+
umfDisjointPoolParamsDestroy(raw_params);
113+
return {nullptr, [](void *) {}};
99114
}
100115

101-
ret = umfDisjointPoolParamsSetMaxPoolableSize(
102-
disjoint_memory_pool_params, 4096 * 16);
103-
116+
ret = umfDisjointPoolParamsSetMaxPoolableSize(raw_params, 4096 * 16);
104117
if (ret != UMF_RESULT_SUCCESS) {
105-
goto err;
118+
state.SkipWithError("Failed to set max poolable size");
119+
umfDisjointPoolParamsDestroy(raw_params);
120+
return {nullptr, [](void *) {}};
106121
}
107-
return;
108-
err:
109-
110-
umfDisjointPoolParamsDestroy(disjoint_memory_pool_params);
111-
disjoint_memory_pool_params = NULL;
112-
}
113122

114-
~disjoint_pool() {
115-
if (disjoint_memory_pool_params != NULL) {
116-
umfDisjointPoolParamsDestroy(disjoint_memory_pool_params);
117-
}
118-
}
123+
// Use a lambda as the deleter
124+
auto deleter = [](void *p) {
125+
if (p) {
126+
umfDisjointPoolParamsDestroy(
127+
static_cast<umf_disjoint_pool_params_handle_t>(p));
128+
}
129+
};
119130

120-
umf_memory_pool_ops_t *
121-
getOps([[maybe_unused]] ::benchmark::State &state) override {
122-
return umfDisjointPoolOps();
131+
return {static_cast<void *>(raw_params), deleter};
123132
}
124-
void *getParams([[maybe_unused]] ::benchmark::State &state) override {
125-
126-
if (disjoint_memory_pool_params == NULL) {
127-
state.SkipWithError("Failed to create disjoint pool params");
128-
}
129133

130-
return disjoint_memory_pool_params;
131-
}
132134
static std::string name() {
133135
return "disjoint_pool<" + Provider::name() + ">";
134136
}
@@ -142,9 +144,7 @@ struct jemalloc_pool : public pool_interface<Provider> {
142144
getOps([[maybe_unused]] ::benchmark::State &state) override {
143145
return umfJemallocPoolOps();
144146
}
145-
void *getParams([[maybe_unused]] ::benchmark::State &state) override {
146-
return NULL;
147-
}
147+
148148
static std::string name() {
149149
return "jemalloc_pool<" + Provider::name() + ">";
150150
}
@@ -158,10 +158,7 @@ struct scalable_pool : public pool_interface<Provider> {
158158
getOps([[maybe_unused]] ::benchmark::State &state) override {
159159
return umfScalablePoolOps();
160160
}
161-
virtual void *
162-
getParams([[maybe_unused]] ::benchmark::State &state) override {
163-
return NULL;
164-
}
161+
165162
static std::string name() {
166163
return "scalable_pool<" + Provider::name() + ">";
167164
}

‎benchmark/benchmark.hpp‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,12 +231,14 @@ class alloc_benchmark : public benchmark_interface<Size, Alloc> {
231231
state.ResumeTiming();
232232
}
233233
}
234+
234235
static std::vector<std::string> argsName() {
235236
auto n = benchmark_interface<Size, Alloc>::argsName();
236237
std::vector<std::string> res = {"max_allocs", "pre_allocs"};
237238
res.insert(res.end(), n.begin(), n.end());
238239
return res;
239240
}
241+
240242
static std::string name() { return base::name() + "/alloc"; }
241243

242244
protected:
@@ -318,12 +320,14 @@ class multiple_malloc_free_benchmark : public alloc_benchmark<Size, Alloc> {
318320
static std::string name() {
319321
return base::base::name() + "/multiple_malloc_free";
320322
}
323+
321324
static std::vector<std::string> argsName() {
322325
auto n = benchmark_interface<Size, Alloc>::argsName();
323326
std::vector<std::string> res = {"max_allocs"};
324327
res.insert(res.end(), n.begin(), n.end());
325328
return res;
326329
}
330+
327331
std::default_random_engine generator;
328332
distribution dist;
329333
};
@@ -349,9 +353,11 @@ class provider_allocator : public allocator_interface {
349353
}
350354
return ptr;
351355
}
356+
352357
void benchFree(void *ptr, size_t size) override {
353358
umfMemoryProviderFree(provider.provider, ptr, size);
354359
}
360+
355361
static std::string name() { return Provider::name(); }
356362

357363
private:
@@ -371,6 +377,7 @@ template <typename Pool> class pool_allocator : public allocator_interface {
371377
virtual void *benchAlloc(size_t size) override {
372378
return umfPoolMalloc(pool.pool, size);
373379
}
380+
374381
virtual void benchFree(void *ptr, [[maybe_unused]] size_t size) override {
375382
umfPoolFree(pool.pool, ptr);
376383
}

‎benchmark/benchmark_interfaces.hpp‎

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
*
77
*/
88

9+
#include <memory>
910
#include <string>
1011
#include <thread>
1112
#include <vector>
@@ -39,6 +40,7 @@ struct benchmark_interface : public benchmark::Fixture {
3940
int argPos = alloc_size.SetUp(state, 0);
4041
allocator.SetUp(state, argPos);
4142
}
43+
4244
void TearDown(::benchmark::State &state) {
4345
alloc_size.TearDown(state);
4446
allocator.TearDown(state);
@@ -54,20 +56,24 @@ struct benchmark_interface : public benchmark::Fixture {
5456
res.insert(res.end(), a.begin(), a.end());
5557
return res;
5658
}
59+
5760
static std::string name() { return Allocator::name(); }
5861

5962
Size alloc_size;
6063
Allocator allocator;
6164
};
6265

6366
struct provider_interface {
67+
using params_ptr = std::unique_ptr<void, void (*)(void *)>;
68+
6469
umf_memory_provider_handle_t provider = NULL;
6570
virtual void SetUp(::benchmark::State &state) {
6671
if (state.thread_index() != 0) {
6772
return;
6873
}
74+
auto params = getParams(state);
6975
auto umf_result =
70-
umfMemoryProviderCreate(getOps(), getParams(), &provider);
76+
umfMemoryProviderCreate(getOps(state), params.get(), &provider);
7177
if (umf_result != UMF_RESULT_SUCCESS) {
7278
state.SkipWithError("umfMemoryProviderCreate() failed");
7379
}
@@ -83,21 +89,30 @@ struct provider_interface {
8389
}
8490
}
8591

86-
virtual umf_memory_provider_ops_t *getOps() { return nullptr; }
87-
virtual void *getParams() { return nullptr; }
92+
virtual umf_memory_provider_ops_t *
93+
getOps([[maybe_unused]] ::benchmark::State &state) {
94+
return nullptr;
95+
}
96+
97+
virtual params_ptr getParams([[maybe_unused]] ::benchmark::State &state) {
98+
return {nullptr, [](void *) {}};
99+
}
88100
};
89101

90102
template <typename T,
91103
typename =
92104
std::enable_if_t<std::is_base_of<provider_interface, T>::value>>
93105
struct pool_interface {
106+
using params_ptr = std::unique_ptr<void, void (*)(void *)>;
107+
94108
virtual void SetUp(::benchmark::State &state) {
95109
provider.SetUp(state);
96110
if (state.thread_index() != 0) {
97111
return;
98112
}
113+
auto params = getParams(state);
99114
auto umf_result = umfPoolCreate(getOps(state), provider.provider,
100-
getParams(state), 0, &pool);
115+
params.get(), 0, &pool);
101116
if (umf_result != UMF_RESULT_SUCCESS) {
102117
state.SkipWithError("umfPoolCreate() failed");
103118
}
@@ -121,8 +136,8 @@ struct pool_interface {
121136
getOps([[maybe_unused]] ::benchmark::State &state) {
122137
return nullptr;
123138
}
124-
virtual void *getParams([[maybe_unused]] ::benchmark::State &state) {
125-
return nullptr;
139+
virtual params_ptr getParams([[maybe_unused]] ::benchmark::State &state) {
140+
return {nullptr, [](void *) {}};
126141
}
127142
T provider;
128143
umf_memory_pool_handle_t pool;

0 commit comments

Comments
 (0)