-
Notifications
You must be signed in to change notification settings - Fork 35
[disjoint] Set default parameters for disjoint pool #1377
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
base: main
Are you sure you want to change the base?
Conversation
src/pool/pool_disjoint.c
Outdated
*params = (umf_disjoint_pool_params_t){ | ||
.slab_min_size = 0, | ||
.max_poolable_size = 0, | ||
.slab_min_size = 64 * 1024, // 64K | ||
.max_poolable_size = 2 * 1024 * 1024, // 2MB | ||
.capacity = 0, | ||
.min_bucket_size = UMF_DISJOINT_POOL_MIN_BUCKET_DEFAULT_SIZE, | ||
.cur_pool_size = 0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please update documentation with default values - for each set function just add info what is default setting
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please check, I've updated details in setter functions.
src/pool/pool_disjoint.c
Outdated
.slab_min_size = 0, | ||
.max_poolable_size = 0, | ||
.slab_min_size = 64 * 1024, // 64K | ||
.max_poolable_size = 2 * 1024 * 1024, // 2MB | ||
.capacity = 0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure if non zero default would be better (1?) @bratpiorka
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please take default settings from here ("host" config): https://github.com/oneapi-src/unified-runtime/blob/568a96aabc6edabe8514ae163aecc64cd5a41878/source/common/umf_pools/disjoint_pool_config_parser.cpp#L50
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
bb6661e
to
805919a
Compare
// This will use the default disjoint pool parameters | ||
void *ptr = umfPoolMalloc(pool, 64); | ||
ASSERT_NE(ptr, nullptr); | ||
ret = umfPoolFree(pool, ptr); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
with the current default params, the umfPoolFree() should not call provider free() - you could check it here by adding a static counter in free()
ASSERT_EQ(ret, UMF_RESULT_SUCCESS); | ||
|
||
// Test allocation and deallocation with a different size | ||
ptr = umfPoolMalloc(pool, 1024); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
change 1024 to a size > max pollable size (e.g. 4MB) and check if the provider free is called at umfPoolFree
Description
fixes #804
Checklist