From dde821806a908f1c8a07f210e5540e08cd6a8f6a Mon Sep 17 00:00:00 2001 From: John Biddiscombe Date: Wed, 23 Mar 2022 21:47:48 +0100 Subject: [PATCH 1/2] Remove an allocation when using register_user_allocation Move the contents of the user_allocation struct into the main block and remove any dupllicated info. By storing the data directly, we do not need a heap allocate a user struct. --- include/hwmalloc/detail/block.hpp | 10 +--- include/hwmalloc/detail/segment.hpp | 4 +- include/hwmalloc/detail/user_allocation.hpp | 62 +++++++-------------- include/hwmalloc/heap.hpp | 16 +++--- 4 files changed, 32 insertions(+), 60 deletions(-) diff --git a/include/hwmalloc/detail/block.hpp b/include/hwmalloc/detail/block.hpp index c94c669..b22c1b0 100644 --- a/include/hwmalloc/detail/block.hpp +++ b/include/hwmalloc/detail/block.hpp @@ -18,9 +18,6 @@ namespace detail template class segment; -template -struct user_allocation; - template struct block_t { @@ -30,11 +27,11 @@ struct block_t using device_handle_type = typename region_traits_type::device_handle_type; #endif using segment_type = segment; - using user_allocation_type = user_allocation; segment_type* m_segment = nullptr; - user_allocation_type* m_user_allocation = nullptr; void* m_ptr = nullptr; + void* m_user_ptr = nullptr; + bool m_user_delete = false; handle_type m_handle; #if HWMALLOC_ENABLE_DEVICE void* m_device_ptr = nullptr; @@ -52,8 +49,7 @@ struct block_t void release() const noexcept { if (m_segment) release_from_segment(); - else if (m_user_allocation) - release_user_allocation(); + else release_user_allocation(); } }; diff --git a/include/hwmalloc/detail/segment.hpp b/include/hwmalloc/detail/segment.hpp index 7c6660c..0f0054a 100644 --- a/include/hwmalloc/detail/segment.hpp +++ b/include/hwmalloc/detail/segment.hpp @@ -84,7 +84,7 @@ class segment char* origin = (char*)m_allocation.m.ptr; for (std::size_t i = m_num_blocks; i > 0; --i) { - block b{this, nullptr, origin + (i - 1) * block_size, + block b{this, origin + (i - 1) * block_size, nullptr, false, m_region.get_handle((i - 1) * block_size, block_size)}; while (!free_stack.push(b)) {} } @@ -109,7 +109,7 @@ class segment char* device_origin = (char*)m_device_allocation.m; for (std::size_t i = m_num_blocks; i > 0; --i) { - block b{this, nullptr, origin + (i - 1) * block_size, + block b{this, origin + (i - 1) * block_size, nullptr, false, m_region.get_handle((i - 1) * block_size, block_size), device_origin + (i - 1) * block_size, m_device_region->get_handle((i - 1) * block_size, block_size), device_id}; diff --git a/include/hwmalloc/detail/user_allocation.hpp b/include/hwmalloc/detail/user_allocation.hpp index 190f236..17900eb 100644 --- a/include/hwmalloc/detail/user_allocation.hpp +++ b/include/hwmalloc/detail/user_allocation.hpp @@ -21,70 +21,42 @@ namespace detail template struct user_allocation { - using region_type = typename detail::region_traits::region_type; + using region_type = decltype(hwmalloc::register_memory(*((Context*)0), nullptr, 0u)); #if HWMALLOC_ENABLE_DEVICE using device_region_type = typename detail::region_traits::device_region_type; #endif using block_type = block_t; using pointer = hw_void_ptr; - struct host_allocation - { - void* m_ptr; - bool m_delete; - host_allocation(void* ptr, bool del) noexcept - : m_ptr{ptr} - , m_delete{del} - { - } - host_allocation(host_allocation const&) noexcept = delete; - host_allocation& operator=(host_allocation const&) noexcept = delete; - - host_allocation(host_allocation&& other) noexcept - : m_ptr{std::exchange(other.m_ptr, nullptr)} - , m_delete{other.m_delete} - { - } - host_allocation& operator=(host_allocation&& other) noexcept - { - if (m_ptr && m_delete) std::free(m_ptr); - m_ptr = std::exchange(other.m_ptr, nullptr); - m_delete = other.m_delete; - return *this; - } - ~host_allocation() - { - if (m_ptr && m_delete) std::free(m_ptr); - } - }; - - //Context* m_context; - host_allocation m_host_allocation; + void* m_host_ptr; + bool m_host_delete; region_type m_region; #if HWMALLOC_ENABLE_DEVICE std::unique_ptr m_device_region; #endif user_allocation(Context* context, void* ptr, std::size_t size) - //: m_context{context} - : m_host_allocation{ptr, false} - , m_region{hwmalloc::register_memory(*context, ptr, size)} + : m_host_ptr{ptr} + , m_host_delete{false} + , m_region{hwmalloc::register_memory(*context, ptr, size)} { } #if HWMALLOC_ENABLE_DEVICE user_allocation(Context* context, void* device_ptr, int /*device_id*/, std::size_t size) - : m_host_allocation{std::malloc(size), true} - , m_region{hwmalloc::register_memory(*context, m_host_allocation.m_ptr, size)} - , m_device_region{std::make_unique( + : m_host_ptr{std::malloc(size)} + , m_host_delete{true} + , m_region{hwmalloc::register_memory(*context, m_host_ptr, size)} + , m_device_region{std::make_unique( hwmalloc::register_device_memory(*context, device_ptr, size))} { } user_allocation(Context* context, void* ptr, void* device_ptr, int /*device_id*/, std::size_t size) - : m_host_allocation{ptr, false} - , m_region{hwmalloc::register_memory(*context, ptr, size)} - , m_device_region{std::make_unique( + : m_host_ptr{ptr} + , m_host_delete{false} + , m_region{hwmalloc::register_memory(*context, ptr, size)} + , m_device_region{std::make_unique( hwmalloc::register_device_memory(*context, device_ptr, size))} { } @@ -95,7 +67,11 @@ template void block_t::release_user_allocation() const noexcept { - delete m_user_allocation; + // do we need to check m_user_ptr? + m_handle.deregister(); + if (m_user_delete) { + std::free(m_user_ptr); + } } } // namespace detail diff --git a/include/hwmalloc/heap.hpp b/include/hwmalloc/heap.hpp index e509251..68bd825 100644 --- a/include/hwmalloc/heap.hpp +++ b/include/hwmalloc/heap.hpp @@ -213,8 +213,8 @@ class heap pointer register_user_allocation(void* ptr, std::size_t size) { - auto a = new detail::user_allocation{m_context, ptr, size}; - return {block_type{nullptr, a, ptr, a->m_region.get_handle(0, size)}}; + detail::user_allocation ua{m_context, ptr, size}; + return {block_type{nullptr, ptr, ptr, false, std::move(ua.m_region)}}; } #if HWMALLOC_ENABLE_DEVICE @@ -241,16 +241,16 @@ class heap pointer register_user_allocation(void* device_ptr, int device_id, std::size_t size) { - auto a = new detail::user_allocation{m_context, device_ptr, device_id, size}; - return {block_type{nullptr, a, a->m_host_allocation.m_ptr, a->m_region.get_handle(0, size), - device_ptr, a->m_device_region->get_handle(0, size), device_id}}; + detail::user_allocation ua{m_context, device_ptr, device_id, size}; + return {block_type{nullptr, device_ptr, ua.m_host_ptr, ua.m_host_delete, std::move(ua.m_region.get_handle(0, size)), + device_ptr, std::move(ua->m_device_region->get_handle(0, size)), device_id}}; } pointer register_user_allocation(void* ptr, void* device_ptr, int device_id, std::size_t size) { - auto a = new detail::user_allocation{m_context, ptr, device_ptr, device_id, size}; - return {block_type{nullptr, a, ptr, a->m_region.get_handle(0, size), device_ptr, - a->m_device_region->get_handle(0, size), device_id}}; + detail::user_allocation ua{m_context, ptr, device_ptr, device_id, size}; + return {block_type{nullptr, device_ptr, ua.m_host_ptr, ua.m_host_delete, std::move(ua->m_region.get_handle(0, size)), + device_ptr, std::move(ua->m_device_region->get_handle(0, size)), device_id}}; } #endif From 53d383c94de4d72d3e32361eff8b5e9577e71441 Mon Sep 17 00:00:00 2001 From: John Biddiscombe Date: Fri, 13 May 2022 11:38:41 +0200 Subject: [PATCH 2/2] Tweak the region handle return to avoid slicing --- include/hwmalloc/heap.hpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/hwmalloc/heap.hpp b/include/hwmalloc/heap.hpp index 68bd825..2bd8e89 100644 --- a/include/hwmalloc/heap.hpp +++ b/include/hwmalloc/heap.hpp @@ -214,7 +214,8 @@ class heap pointer register_user_allocation(void* ptr, std::size_t size) { detail::user_allocation ua{m_context, ptr, size}; - return {block_type{nullptr, ptr, ptr, false, std::move(ua.m_region)}}; + auto p = block_type{nullptr, ptr, ptr, false, ua.m_region.get_handle(0,size)}; + return {p}; } #if HWMALLOC_ENABLE_DEVICE