-
Notifications
You must be signed in to change notification settings - Fork 22
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Allocator handle for custom allocators used in shared library #84
Allocator handle for custom allocators used in shared library #84
Conversation
ad1a520
to
cb8f09c
Compare
include/svs/core/allocator.h
Outdated
/// | ||
/// AllocatorHandle | ||
/// ================ | ||
/// `AllocatorHandle` is specifically designed to support LVQ/Leanvec Datasets with a custom |
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.
Is there something unique about the AllocatorHandle when it comes to LVQ/Leanvec datasets? From what I understand, its primary use case is to enable these custom datasets to work with a shared library. However, I believe that the AllocatorHandle is more versatile and can be applied to a variety of other scenarios as well. Is that right @dian-lun-lin ?
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.
Yes, AllocatorHandle
can be applied to a broader range of scenarios. I initially focused on its use with LVQ/LeanVec, but I will update the documentation to reflect its applicability. Thank you!
if (impl_.get() == nullptr) { | ||
throw ANNEXCEPTION("Empty allocator handle!"); | ||
} | ||
return static_cast<T*>(impl_->allocate(n)); |
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.
IMHO, to avoid misusage, would be good to add requirements same_as<T, Impl::value_type>
, elsewhere there is the risk of wrong amount of bytes allocation.
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.
That's a good suggestion. I've added the requirement in the constructor. Thank you.
impl_.deallocate(static_cast<typename Impl::value_type*>(ptr), n); | ||
} | ||
|
||
AllocatorImpl<Impl>* clone() const override { return new AllocatorImpl(impl_); } |
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.
As I understand, this means that Impl
type should be 'copyable', i.e. Impl::Impl(const Impl& other)
or Impl::Impl(Impl other)
should be defined, right?
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.
Yes, I've added the requirement in the Allocator
concept. Thanks!
include/svs/core/allocator.h
Outdated
: impl_{other.impl_->clone()} {} | ||
AllocatorHandle(AllocatorHandle&&) = default; | ||
AllocatorHandle& operator=(const AllocatorHandle& other) { | ||
impl_ = other.impl_->clone(); |
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.
nitpick: different allocation methods for AllocatorImpl
:
- In
AllocatorHandle
.ctor usedmake_unique(...)
- When here is:
unique_ptr::unique_ptr(new ...)
I also do not understand, why C++ compiler allows this construction which looks like:
std::unique_ptr<AllocatorInterface>::operator=(AllocatorInterface*);
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.
I've updated the code to use reset()
. Thanks for catching that!
This PR is specifically designed to support a custom allocator that works with a shared library. It offers an interface that enables the implementation of a custom allocator not included in the shared library. Which custom allocator to use is determined at runtime.
This PR provides three utility functions:
make_allocator_handle(Alloc alloc)
: Accepts an allocator and returns anAllocatorHandle
to wrap the allocator.make_blocked_allocator_handle(Alloc alloc)
: Accepts an allocator, wraps it in anAllocatorHandle
, and returns aBlocked
object that manages the allocator.make_blocked_allocator_handle(const BlockingParameters& parameters, Alloc alloc)
: Accepts an allocator, wraps it in anAllocatorHandle
, and returns aBlocked
object configured with the specified parameters.The
AllocatorHandle
providesallocate
anddeallocate
functions to use the wrapped custom allocator for memory allocation.