Better allocator support #2
Conversation
… Umpire for SplitVectors
…stalled and found
| @@ -85,8 +95,7 @@ class Hashmap { | |||
| void preallocate_device_handles() { | |||
| #ifndef HASHINATOR_CPU_ONLY_MODE | |||
| SPLIT_CHECK_ERR(split_gpuMalloc((void**)&device_map, sizeof(Hashmap))); | |||
| device_buckets = reinterpret_cast<split::SplitVector<hash_pair<KEY_TYPE, VAL_TYPE>>*>( | |||
| reinterpret_cast<char*>(device_map) + offsetof(Hashmap, buckets)); | |||
| device_buckets = &device_map->buckets; | |||
There was a problem hiding this comment.
Is this in fact now doing the same thing and usint the arrow operator after the address in order to get an offsetof? Because if it isn't, then this would be trying to store a device-side value to the host here.
| @@ -1214,6 +1246,24 @@ class Hashmap { | |||
| } | |||
| return elements.size(); | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
up - what I mean is that with Allocator support, will the old versions (which do not template allocators) ever be called anymore? Or could they be safely deleted?
| defaults::WARPSIZE>(buckets, elements, rule, stack, max_size, s); | ||
| return elements.size(); | ||
| } | ||
|
|
| return std::nullopt; | ||
| } | ||
| } | ||
| return std::nullopt; |
There was a problem hiding this comment.
I see you changed assertions to be a nullopt. How would the user catch that?
There was a problem hiding this comment.
If the user tries to access this nullopt a bad access exception will be thrown had the key not been in the map. But I really do not like this change so I take it back.
| *this = SplitVector<T, Allocator>(1, std::move(val)); | ||
| return; | ||
| } | ||
| // if (_data == nullptr) { |
| @@ -322,8 +355,8 @@ class Hashmap { | |||
| // DeviceHasher::reset_all(buckets.data(),_mapInfo, buckets.size(), s); | |||
There was a problem hiding this comment.
Posting comment here: On line 326, the mallocation of a temporary buffer should surely happen with the provided allocator.
Same on line 1339 for tombstone cleaning.
So in this PR there is some pruning in the SplitVector code to enable the use of external allocators such as Umpire. This PR enables Hashinator as well to propagate the allocators to SplitVector. A citation methods has also been added and as well as some changes to the README.
split::split_host_allocatoris not needed anymore an replace withstd::allocatorsizeof(T)is enormous this approach is not efficient, but since Hashinator and SplitVectors are meant to be used with trivial types this is rather unlikely.