Skip to content

Commit d4d7f89

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 d4d7f89

File tree

3 files changed

+84
-66
lines changed

3 files changed

+84
-66
lines changed

‎benchmark/benchmark.cpp‎

Lines changed: 58 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -37,20 +37,30 @@ 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 provider_interface::params_ptr(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 provider_interface::params_ptr(static_cast<void *>(raw_params),
59+
deleter);
5060
}
5161

52-
void *getParams() override { return params; }
53-
umf_memory_provider_ops_t *getOps() override {
62+
umf_memory_provider_ops_t *
63+
getOps([[maybe_unused]] ::benchmark::State &state) override {
5464
return umfOsMemoryProviderOps();
5565
}
5666
static std::string name() { return "os_provider"; }
@@ -62,73 +72,66 @@ struct proxy_pool : public pool_interface<Provider> {
6272
getOps([[maybe_unused]] ::benchmark::State &state) override {
6373
return umfProxyPoolOps();
6474
}
65-
void *getParams([[maybe_unused]] ::benchmark::State &state) override {
66-
return nullptr;
67-
}
75+
6876
static std::string name() { return "proxy_pool<" + Provider::name() + ">"; }
6977
};
7078

7179
#ifdef UMF_POOL_DISJOINT_ENABLED
7280
template <typename Provider>
7381
struct disjoint_pool : public pool_interface<Provider> {
74-
umf_disjoint_pool_params_handle_t disjoint_memory_pool_params;
82+
umf_memory_pool_ops_t *
83+
getOps([[maybe_unused]] ::benchmark::State &state) override {
84+
return umfDisjointPoolOps();
85+
}
7586

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

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

90-
ret = umfDisjointPoolParamsSetCapacity(disjoint_memory_pool_params, 4);
103+
ret = umfDisjointPoolParamsSetCapacity(raw_params, 4);
91104
if (ret != UMF_RESULT_SUCCESS) {
92-
goto err;
105+
state.SkipWithError("Failed to set capacity");
106+
umfDisjointPoolParamsDestroy(raw_params);
107+
return {nullptr, [](void *) {}};
93108
}
94109

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

101-
ret = umfDisjointPoolParamsSetMaxPoolableSize(
102-
disjoint_memory_pool_params, 4096 * 16);
103-
117+
ret = umfDisjointPoolParamsSetMaxPoolableSize(raw_params, 4096 * 16);
104118
if (ret != UMF_RESULT_SUCCESS) {
105-
goto err;
119+
state.SkipWithError("Failed to set max poolable size");
120+
umfDisjointPoolParamsDestroy(raw_params);
121+
return {nullptr, [](void *) {}};
106122
}
107-
return;
108-
err:
109123

110-
umfDisjointPoolParamsDestroy(disjoint_memory_pool_params);
111-
disjoint_memory_pool_params = NULL;
112-
}
124+
// Use a lambda as the deleter
125+
auto deleter = [](void *p) {
126+
if (p) {
127+
umfDisjointPoolParamsDestroy(
128+
static_cast<umf_disjoint_pool_params_handle_t>(p));
129+
}
130+
};
113131

114-
~disjoint_pool() {
115-
if (disjoint_memory_pool_params != NULL) {
116-
umfDisjointPoolParamsDestroy(disjoint_memory_pool_params);
117-
}
132+
return {static_cast<void *>(raw_params), deleter};
118133
}
119134

120-
umf_memory_pool_ops_t *
121-
getOps([[maybe_unused]] ::benchmark::State &state) override {
122-
return umfDisjointPoolOps();
123-
}
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-
}
129-
130-
return disjoint_memory_pool_params;
131-
}
132135
static std::string name() {
133136
return "disjoint_pool<" + Provider::name() + ">";
134137
}
@@ -142,9 +145,7 @@ struct jemalloc_pool : public pool_interface<Provider> {
142145
getOps([[maybe_unused]] ::benchmark::State &state) override {
143146
return umfJemallocPoolOps();
144147
}
145-
void *getParams([[maybe_unused]] ::benchmark::State &state) override {
146-
return NULL;
147-
}
148+
148149
static std::string name() {
149150
return "jemalloc_pool<" + Provider::name() + ">";
150151
}
@@ -158,10 +159,7 @@ struct scalable_pool : public pool_interface<Provider> {
158159
getOps([[maybe_unused]] ::benchmark::State &state) override {
159160
return umfScalablePoolOps();
160161
}
161-
virtual void *
162-
getParams([[maybe_unused]] ::benchmark::State &state) override {
163-
return NULL;
164-
}
162+
165163
static std::string name() {
166164
return "scalable_pool<" + Provider::name() + ">";
167165
}

‎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: 19 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>
@@ -61,13 +62,16 @@ struct benchmark_interface : public benchmark::Fixture {
6162
};
6263

6364
struct provider_interface {
65+
using params_ptr = std::unique_ptr<void, void (*)(void *)>;
66+
6467
umf_memory_provider_handle_t provider = NULL;
6568
virtual void SetUp(::benchmark::State &state) {
6669
if (state.thread_index() != 0) {
6770
return;
6871
}
72+
auto params = getParams(state);
6973
auto umf_result =
70-
umfMemoryProviderCreate(getOps(), getParams(), &provider);
74+
umfMemoryProviderCreate(getOps(state), params.get(), &provider);
7175
if (umf_result != UMF_RESULT_SUCCESS) {
7276
state.SkipWithError("umfMemoryProviderCreate() failed");
7377
}
@@ -83,21 +87,30 @@ struct provider_interface {
8387
}
8488
}
8589

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

90100
template <typename T,
91101
typename =
92102
std::enable_if_t<std::is_base_of<provider_interface, T>::value>>
93103
struct pool_interface {
104+
using params_ptr = std::unique_ptr<void, void (*)(void *)>;
105+
94106
virtual void SetUp(::benchmark::State &state) {
95107
provider.SetUp(state);
96108
if (state.thread_index() != 0) {
97109
return;
98110
}
111+
auto params = getParams(state);
99112
auto umf_result = umfPoolCreate(getOps(state), provider.provider,
100-
getParams(state), 0, &pool);
113+
params.get(), 0, &pool);
101114
if (umf_result != UMF_RESULT_SUCCESS) {
102115
state.SkipWithError("umfPoolCreate() failed");
103116
}
@@ -121,8 +134,8 @@ struct pool_interface {
121134
getOps([[maybe_unused]] ::benchmark::State &state) {
122135
return nullptr;
123136
}
124-
virtual void *getParams([[maybe_unused]] ::benchmark::State &state) {
125-
return nullptr;
137+
virtual params_ptr getParams([[maybe_unused]] ::benchmark::State &state) {
138+
return params_ptr(nullptr, [](void *) {});
126139
}
127140
T provider;
128141
umf_memory_pool_handle_t pool;

0 commit comments

Comments
 (0)