add GetCacheLineSize() op to memory provider and use it to initialize DP min bucket size#1587
add GetCacheLineSize() op to memory provider and use it to initialize DP min bucket size#1587bratpiorka wants to merge 2 commits into
Conversation
2dad996 to
9fe2597
Compare
a287a16 to
63b0991
Compare
|
|
||
| auto expected_bucket_size = [min_bucket_size](size_t i) -> size_t { | ||
| // Even indexes: min,2*min,4*min,... | ||
| // Odd indexes: 1.5*min,3*min,6*min,... |
There was a problem hiding this comment.
ok, I have concerns about the 1.5 * min case since I don't see a requirement for cache_line_size alignment on the front of the array, wouldn't 1.5 * min result in the end of a buffer being halfway through a cache line ? and then the subsequent allocation would start there (causing two allocations to share a cache line? )
I suggest one of two options:
- actually enforce starting on min_bucket_size alignment (which is now consistent with cache_line_size)
- delete 1.5 * min, so you have Even indices: min, 2 * min, 4 * min ... and Odd Indices: (1.5 * min ->2 * min or deleted), 3 * min, 6 * min ...
| ? (min_bucket_size << (i / 2)) | ||
| : ((min_bucket_size + min_bucket_size / 2) << (i / 2)); | ||
| }; | ||
|
|
There was a problem hiding this comment.
The implementation of 2 could be enforced using something like
// stays the same or rounds up to the next multiple of pow_2
// where pow_2 should be a power of two
template<typename T, typename std::enable_if<std::is_integral<T>::value>::type* = nullptr>
T round_up_next_multiple(T count, T pow_2_mult)
{
assert(pow_2_mult != 0 && ((pow_2_mult & pow_2_mult - 1) == 0)); // check pow_2_mult is a power of two
return (count + pow_2_mult - 1) & ~(pow_2_mult - 1);
}
// enforce length is a multiple of min_bucket_size, which in round about way enables alignment for subsequent allocations to be on min_bucket_size (aka cache_line_size)
expected_bucket_size = round_up_next_multiple<size_t>(expected_bucket_size, min_bucket_size);
There was a problem hiding this comment.
or something like that with a lambda ... or as the bad pattern is only for the case of "i==3", you could try to modify your ternary into something more complicated. I kind of like the use of this "helper" function since it signals the intent very clearly, even if it is five extra OPs ...
| size_t cacheLineSize = 0; | ||
| umf_result = umfMemoryProviderGetCacheLineSize(provider, &cacheLineSize); | ||
| EXPECT_EQ(umf_result, UMF_RESULT_SUCCESS); | ||
| EXPECT_GT(cacheLineSize, 0); |
There was a problem hiding this comment.
Shouldn't it be equal to the default of 128 for a level zero memory provider?
add GetCacheLineSize() op to memory provider and use it to initialize DP min bucket size