From 794a5912c7fd545c9f8c060a4a885552a2096dd2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20Lie=CC=81tar?= Date: Thu, 9 Apr 2020 15:46:01 +0200 Subject: [PATCH 1/3] Replace uses of std::function by function_ref. --- src/ds/helpers.h | 36 ++++++++++++++++++++++++++++++++++++ src/mem/alloc.h | 27 +++++++++++---------------- src/mem/globalalloc.h | 2 +- src/mem/threadalloc.h | 9 ++++----- 4 files changed, 52 insertions(+), 22 deletions(-) diff --git a/src/ds/helpers.h b/src/ds/helpers.h index e3c2290..979c4b6 100644 --- a/src/ds/helpers.h +++ b/src/ds/helpers.h @@ -101,4 +101,40 @@ namespace snmalloc f(); } }; + + /** + * Non-owning version of std::function. Wraps a reference to a callable object + * (eg. a lambda) and allows calling it through dynamic dispatch, with no + * allocation. This is useful in the allocator code paths, where we can't + * safely use std::function. + * + * Inspired off the C++ proposal: + * http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0792r2.html + */ + template + struct function_ref; + template + struct function_ref + { + template + function_ref(Fn&& fn) + { + data_ = static_cast(&fn); + fn_ = execute; + } + R operator()(Args... args) const + { + return fn_(data_, args...); + } + + private: + void* data_; + R (*fn_)(void*, Args...); + + template + static R execute(void* p, Args... args) + { + return (*static_cast>(p))(args...); + }; + }; } // namespace snmalloc diff --git a/src/mem/alloc.h b/src/mem/alloc.h index f5dc2e5..bf389b4 100644 --- a/src/mem/alloc.h +++ b/src/mem/alloc.h @@ -83,7 +83,7 @@ namespace snmalloc */ template< bool (*NeedsInitialisation)(void*), - void* (*InitThreadAllocator)(std::function&), + void* (*InitThreadAllocator)(function_ref), class MemoryProvider = GlobalVirtual, class ChunkMap = SNMALLOC_DEFAULT_CHUNKMAP, bool IsQueueInline = true> @@ -1141,10 +1141,9 @@ namespace snmalloc template SNMALLOC_SLOW_PATH void* small_alloc_first_alloc(size_t size) { - std::function f = [size](void* alloc) { + return InitThreadAllocator([size](void* alloc) { return reinterpret_cast(alloc)->alloc(size); - }; - return InitThreadAllocator(f); + }); } /** @@ -1321,10 +1320,9 @@ namespace snmalloc { if (NeedsInitialisation(this)) { - std::function f = [size](void* alloc) { + return InitThreadAllocator([size](void* alloc) { return reinterpret_cast(alloc)->alloc(size); - }; - return InitThreadAllocator(f); + }); } slab = reinterpret_cast( large_allocator.template alloc( @@ -1395,10 +1393,9 @@ namespace snmalloc if (NeedsInitialisation(this)) { - std::function f = [size](void* alloc) { + return InitThreadAllocator([size](void* alloc) { return reinterpret_cast(alloc)->alloc(size); - }; - return InitThreadAllocator(f); + }); } size_t size_bits = bits::next_pow2_bits(size); @@ -1423,11 +1420,10 @@ namespace snmalloc if (NeedsInitialisation(this)) { - std::function f = [p](void* alloc) { + InitThreadAllocator([p](void* alloc) { reinterpret_cast(alloc)->dealloc(p); return nullptr; - }; - InitThreadAllocator(f); + }); return; } @@ -1479,11 +1475,10 @@ namespace snmalloc // a real allocator and construct one if we aren't. if (NeedsInitialisation(this)) { - std::function f = [p](void* alloc) { + InitThreadAllocator([p](void* alloc) { reinterpret_cast(alloc)->dealloc(p); return nullptr; - }; - InitThreadAllocator(f); + }); return; } diff --git a/src/mem/globalalloc.h b/src/mem/globalalloc.h index 37c75dc..dac401f 100644 --- a/src/mem/globalalloc.h +++ b/src/mem/globalalloc.h @@ -7,7 +7,7 @@ namespace snmalloc { inline bool needs_initialisation(void*); - void* init_thread_allocator(std::function&); + void* init_thread_allocator(function_ref); template class AllocPool : Pool diff --git a/src/mem/threadalloc.h b/src/mem/threadalloc.h index 47653f0..ba8d2fa 100644 --- a/src/mem/threadalloc.h +++ b/src/mem/threadalloc.h @@ -62,7 +62,7 @@ namespace snmalloc # pragma warning(push) # pragma warning(disable : 4702) # endif - SNMALLOC_FAST_PATH void* init_thread_allocator(std::function& f) + SNMALLOC_FAST_PATH void* init_thread_allocator(function_ref f) { error("Critical Error: This should never be called."); return f(nullptr); @@ -101,7 +101,7 @@ namespace snmalloc */ class ThreadAllocCommon { - friend void* init_thread_allocator(std::function&); + friend void* init_thread_allocator(function_ref); protected: /** @@ -195,12 +195,11 @@ namespace snmalloc auto*& alloc = get_reference(); if (unlikely(needs_initialisation(alloc)) && !destructor_has_run) { - std::function f = [](void*) { return nullptr; }; // Call `init_thread_allocator` to perform down call in case // register_clean_up does more. // During teardown for the destructor based ThreadAlloc this will set // alloc to GlobalPlaceHolder; - init_thread_allocator(f); + init_thread_allocator([](void*) { return nullptr; }); } return alloc; # endif @@ -277,7 +276,7 @@ namespace snmalloc * path. * The second component of the return indicates if this TLS is being torndown. */ - SNMALLOC_FAST_PATH void* init_thread_allocator(std::function& f) + SNMALLOC_FAST_PATH void* init_thread_allocator(function_ref f) { auto*& local_alloc = ThreadAlloc::get_reference(); // If someone reuses a noncachable call, then we can end up here From 1ccb808a186d4dd8ac817170a80ce62301a11125 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20Lie=CC=81tar?= Date: Fri, 10 Apr 2020 12:36:36 +0200 Subject: [PATCH 2/3] Fix clang-tidy warning and CR comments. --- src/ds/helpers.h | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/ds/helpers.h b/src/ds/helpers.h index 979c4b6..00159f8 100644 --- a/src/ds/helpers.h +++ b/src/ds/helpers.h @@ -108,7 +108,7 @@ namespace snmalloc * allocation. This is useful in the allocator code paths, where we can't * safely use std::function. * - * Inspired off the C++ proposal: + * Inspired by the C++ proposal: * http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0792r2.html */ template @@ -116,12 +116,17 @@ namespace snmalloc template struct function_ref { - template - function_ref(Fn&& fn) + // The enable_if is used to stop this constructor from shadowing the default + // copy / move constructors. + template< + typename Fn, + typename = std::enable_if_t< + !std::is_same_v, function_ref>> function_ref(Fn&& fn) { data_ = static_cast(&fn); fn_ = execute; } + R operator()(Args... args) const { return fn_(data_, args...); From 6f697e06efec9bea61264d7f3f7805322e5e415b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20Lie=CC=81tar?= Date: Fri, 10 Apr 2020 12:54:09 +0200 Subject: [PATCH 3/3] Add missing closing > --- src/ds/helpers.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/ds/helpers.h b/src/ds/helpers.h index 00159f8..c2f2ead 100644 --- a/src/ds/helpers.h +++ b/src/ds/helpers.h @@ -120,8 +120,9 @@ namespace snmalloc // copy / move constructors. template< typename Fn, - typename = std::enable_if_t< - !std::is_same_v, function_ref>> function_ref(Fn&& fn) + typename = + std::enable_if_t, function_ref>>> + function_ref(Fn&& fn) { data_ = static_cast(&fn); fn_ = execute;