From f18d8770e58dfd6f3b67418b55336316207b39dc Mon Sep 17 00:00:00 2001 From: Nathaniel Filardo Date: Mon, 7 Sep 2020 11:56:05 +0100 Subject: [PATCH 1/8] ds/aba: make Cmp constructor explicit For reasons unknown, this appeases MSVC using C++20. --- src/ds/aba.h | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/ds/aba.h b/src/ds/aba.h index fc8e3f8f7..5d4e092b7 100644 --- a/src/ds/aba.h +++ b/src/ds/aba.h @@ -85,6 +85,13 @@ namespace snmalloc Linked old; ABA* parent; + /* + * MSVC apparently does not like the implicit constructor it creates when + * asked to interpret its input as C++20; it rejects the construction up + * in read(), above. Help it out by making the constructor explicit. + */ + Cmp(Linked old, ABA* parent) : old(old), parent(parent) {} + T* ptr() { return old.ptr; From d8a769ae32ff9448f3a5102e94ee074e06228d10 Mon Sep 17 00:00:00 2001 From: Nathaniel Filardo Date: Wed, 2 Sep 2020 12:45:50 +0100 Subject: [PATCH 2/8] AddressSpaceManager: template parameter "PAL" not "Pal" "Pal" is a global symbol for the current architecture's platform abstraction layer class (see src/pal/pal.h); to be less confusing, don't shadow it with a template parameter on the AddressSpaceManager class, and instead use "PAL" as is done elsewhere for template arguments. --- src/mem/address_space.h | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/mem/address_space.h b/src/mem/address_space.h index 2b41ac7b8..a727f0b7b 100644 --- a/src/mem/address_space.h +++ b/src/mem/address_space.h @@ -13,8 +13,8 @@ namespace snmalloc * It cannot unreserve memory, so this does not require the * usual complexity of a buddy allocator. */ - template - class AddressSpaceManager : public Pal + template + class AddressSpaceManager : public PAL { /** * Stores the blocks of address space @@ -160,7 +160,7 @@ namespace snmalloc auto page_start = pointer_align_down(base); auto page_end = pointer_align_up(pointer_offset(base, size)); - Pal::template notify_using( + PAL::template notify_using( page_start, static_cast(page_end - page_start)); } @@ -178,10 +178,10 @@ namespace snmalloc SNMALLOC_ASSERT(bits::next_pow2(size) == size); SNMALLOC_ASSERT(size >= sizeof(void*)); - if constexpr (pal_supports) + if constexpr (pal_supports) { - if (size >= Pal::minimum_alloc_size) - return static_cast(this)->template reserve_aligned( + if (size >= PAL::minimum_alloc_size) + return static_cast(this)->template reserve_aligned( size); } @@ -194,20 +194,20 @@ namespace snmalloc // Allocation failed ask OS for more memory void* block; size_t block_size; - if constexpr (pal_supports) + if constexpr (pal_supports) { - block_size = Pal::minimum_alloc_size; - block = static_cast(this)->template reserve_aligned( + block_size = PAL::minimum_alloc_size; + block = static_cast(this)->template reserve_aligned( block_size); } else { // Need at least 2 times the space to guarantee alignment. // Hold lock here as a race could cause additional requests to - // the Pal, and this could lead to suprious OOM. This is - // particularly bad if the Pal gives all the memory on first call. + // the PAL, and this could lead to suprious OOM. This is + // particularly bad if the PAL gives all the memory on first call. auto block_and_size = - static_cast(this)->reserve_at_least(size * 2); + static_cast(this)->reserve_at_least(size * 2); block = block_and_size.first; block_size = block_and_size.second; From 5b6e98c176326a96a95d7163b1c8c4c543393c74 Mon Sep 17 00:00:00 2001 From: Nathaniel Filardo Date: Wed, 2 Sep 2020 12:41:09 +0100 Subject: [PATCH 3/8] Add C++ concept for PAL This will not be used unless the C++ standard version is raised to 20. As concepts and C++20 more generally are quite new, this does not do so. Nevertheless, the use of concepts can improve the local development experience as type mismatches are discovered earlier (at template invocation rather than only during expansion). --- CMakeLists.txt | 3 ++ src/ds/concept.h | 40 ++++++++++++++++ src/mem/address_space.h | 2 +- src/mem/largealloc.h | 6 +-- src/mem/pool.h | 3 +- src/pal/pal.h | 4 +- src/pal/pal_concept.h | 104 ++++++++++++++++++++++++++++++++++++++++ 7 files changed, 156 insertions(+), 6 deletions(-) create mode 100644 src/ds/concept.h create mode 100644 src/pal/pal_concept.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 611d7302e..7191f6373 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -71,6 +71,9 @@ macro(clangformat_targets) else () message(STATUS "Generating clangformat target using ${CLANG_FORMAT}") file(GLOB_RECURSE ALL_SOURCE_FILES *.cc *.h *.hh) + # clangformat does not yet understand concepts well; for the moment, don't + # ask it to format them. See https://reviews.llvm.org/D79773 + list(FILTER ALL_SOURCE_FILES EXCLUDE REGEX "src/pal/pal_concept\.h$") add_custom_target( clangformat COMMAND ${CLANG_FORMAT} diff --git a/src/ds/concept.h b/src/ds/concept.h new file mode 100644 index 000000000..d935417d9 --- /dev/null +++ b/src/ds/concept.h @@ -0,0 +1,40 @@ +#pragma once + +/** + * C++20 concepts are referenced as if they were types in declarations within + * template parameters (e.g. "template ..."). That is, they + * take the place of the "typename"/"class" keyword on template parameters. + * If the compiler understands concepts, this macro expands as its argument; + * otherwise, it expands to the keyword "typename", so snmalloc templates that + * use concept-qualified parameters should use this to remain compatible across + * C++ versions: "template" + */ +#ifdef __cpp_concepts +# define SNMALLOC_CONCEPT(c) c +#else +# define SNMALLOC_CONCEPT(c) typename +#endif + +#ifdef __cpp_concepts +namespace snmalloc +{ + /** + * C++20 concepts are more than just new syntax; there's a new support + * library specified as well. As C++20 is quite new, however, there are some + * environments, notably Clang, that understand the syntax but do not yet + * offer the library. Fortunately, alternate pronouciations are possible. + */ +# ifdef _cpp_lib_concepts + /** + * ConceptSame is true if T and U are the same type and false otherwise. + * When specifying a concept, use ConceptSame to indicate that an + * expression must evaluate precisely to the type U. + */ + template + concept ConceptSame = std::same_as; +# else + template + concept ConceptSame = std::is_same::value; +# endif +} // namespace snmalloc +#endif diff --git a/src/mem/address_space.h b/src/mem/address_space.h index a727f0b7b..00dc21b4f 100644 --- a/src/mem/address_space.h +++ b/src/mem/address_space.h @@ -13,7 +13,7 @@ namespace snmalloc * It cannot unreserve memory, so this does not require the * usual complexity of a buddy allocator. */ - template + template class AddressSpaceManager : public PAL { /** diff --git a/src/mem/largealloc.h b/src/mem/largealloc.h index 2a0c7b3af..ff85bb56c 100644 --- a/src/mem/largealloc.h +++ b/src/mem/largealloc.h @@ -14,7 +14,7 @@ namespace snmalloc { - template + template class MemoryProviderStateMixin; class Largeslab : public Baseslab @@ -24,7 +24,7 @@ namespace snmalloc private: template friend class MPMCStack; - template + template friend class MemoryProviderStateMixin; std::atomic next; @@ -56,7 +56,7 @@ namespace snmalloc // This represents the state that the large allcoator needs to add to the // global state of the allocator. This is currently stored in the memory // provider, so we add this in. - template + template class MemoryProviderStateMixin : public PalNotificationObject, public PAL { /** diff --git a/src/mem/pool.h b/src/mem/pool.h index c460567ed..69de2ca7a 100644 --- a/src/mem/pool.h +++ b/src/mem/pool.h @@ -2,6 +2,7 @@ #include "../ds/flaglock.h" #include "../ds/mpmcstack.h" +#include "../pal/pal_concept.h" #include "pooled.h" namespace snmalloc @@ -21,7 +22,7 @@ namespace snmalloc { private: friend Pooled; - template + template friend class MemoryProviderStateMixin; std::atomic_flag lock = ATOMIC_FLAG_INIT; diff --git a/src/pal/pal.h b/src/pal/pal.h index 47f30e025..9ce3ffc3e 100644 --- a/src/pal/pal.h +++ b/src/pal/pal.h @@ -1,5 +1,7 @@ #pragma once +#include "../ds/concept.h" +#include "pal_concept.h" #include "pal_consts.h" // If simultating OE, then we need the underlying platform @@ -63,7 +65,7 @@ namespace snmalloc /** * Query whether the PAL supports a specific feature. */ - template + template constexpr static bool pal_supports = (PAL::pal_features & F) == F; // Used to keep Superslab metadata committed. diff --git a/src/pal/pal_concept.h b/src/pal/pal_concept.h new file mode 100644 index 000000000..33f54f4cd --- /dev/null +++ b/src/pal/pal_concept.h @@ -0,0 +1,104 @@ +#pragma once + +#ifdef __cpp_concepts +# include "../ds/concept.h" +# include "pal_consts.h" + +# include + +namespace snmalloc +{ + /** + * PALs must advertize the bit vector of their supported features and the + * platform's page size. This concept enforces that these are indeed + * constants that fit in the desired types. (This is subtly different from + * saying that they are the required types; C++ may handle constants without + * much regard for their claimed type.) + */ + template + concept ConceptPAL_static_members = requires() + { + typename std::integral_constant; + typename std::integral_constant; + }; + + /** + * PALs expose an error reporting function which takes a const C string. + */ + template + concept ConceptPAL_error = requires(const char* const str) + { + { PAL::error(str) } -> ConceptSame; + }; + + /** + * PALs expose a basic library of memory operations. + */ + template + concept ConceptPAL_memops = requires(PAL p, void* vp, size_t sz) + { + { p.notify_not_using(vp, sz) } noexcept -> ConceptSame; + + /* For reasons unknown, these seem to tickle some bug in MSVC */ +# if !defined(_MSC_VER) + { p.template notify_using(vp, sz) } noexcept + -> ConceptSame; + { p.template notify_using(vp, sz) } noexcept + -> ConceptSame; +# endif + + { p.template zero(vp, sz) } noexcept -> ConceptSame; + { p.template zero(vp, sz) } noexcept -> ConceptSame; + }; + + /** + * Absent any feature flags, the PAL must support a crude primitive allocator + */ + template + concept ConceptPAL_reserve_at_least = requires(PAL p, void* vp, size_t sz) + { + { p.reserve_at_least(sz) } noexcept + -> ConceptSame>; + }; + + /** + * Some PALs expose a richer allocator which understands aligned allocations + */ + template + concept ConceptPAL_reserve_aligned = requires(PAL p, size_t sz) + { + { p.template reserve_aligned(sz) } noexcept -> ConceptSame; + { p.template reserve_aligned(sz) } noexcept -> ConceptSame; + }; + + /** + * Some PALs can provide memory pressure callbacks. + */ + template + concept ConceptPAL_mem_low_notify = + requires(PAL p, PalNotificationObject* pno) + { + { p.expensive_low_memory_check() } -> ConceptSame; + { p.register_for_low_memory_callback(pno) } -> ConceptSame; + }; + + /** + * PALs ascribe to the conjunction of several concepts. These are broken + * out by the shape of the requires() quantifiers required and by any + * requisite claimed pal_features. PALs not claiming particular features + * are, naturally, not bound by the corresponding concept. + */ + template + concept ConceptPAL = + ConceptPAL_static_members && + ConceptPAL_error && + ConceptPAL_memops && + (!(PAL::pal_features & LowMemoryNotification) || + ConceptPAL_mem_low_notify) && + (!!(PAL::pal_features & AlignedAllocation) || + ConceptPAL_reserve_at_least) && + (!(PAL::pal_features & AlignedAllocation) || + ConceptPAL_reserve_aligned); + +} // namespace snmalloc +#endif From c7a2ebc7489eef1f2530b78d8f0c48fc40dbcc41 Mon Sep 17 00:00:00 2001 From: Nathaniel Filardo Date: Fri, 4 Sep 2020 15:29:51 +0100 Subject: [PATCH 4/8] pal_windows: defer registration for low-mem until use If we're going to explore fully-static PALs, then we shouldn't need a constructor. --- src/pal/pal_windows.h | 49 ++++++++++++++++++++----------------------- 1 file changed, 23 insertions(+), 26 deletions(-) diff --git a/src/pal/pal_windows.h b/src/pal/pal_windows.h index b4cec0989..1e6215ff6 100644 --- a/src/pal/pal_windows.h +++ b/src/pal/pal_windows.h @@ -46,32 +46,6 @@ namespace snmalloc } public: - PALWindows() - { - // No error handling here - if this doesn't work, then we will just - // consume more memory. There's nothing sensible that we could do in - // error handling. We also leak both the low memory notification object - // handle and the wait object handle. We'll need them until the program - // exits, so there's little point doing anything else. - // - // We only try to register once. If this fails, give up. Even if we - // create multiple PAL objects, we don't want to get more than one - // callback. - if (!registered_for_notifications.exchange(true)) - { - lowMemoryObject = - CreateMemoryResourceNotification(LowMemoryResourceNotification); - HANDLE waitObject; - RegisterWaitForSingleObject( - &waitObject, - lowMemoryObject, - low_memory, - nullptr, - INFINITE, - WT_EXECUTEDEFAULT); - } - } - /** * Bitmap of PalFeatures flags indicating the optional features that this * PAL supports. This PAL supports low-memory notifications. @@ -105,6 +79,29 @@ namespace snmalloc static void register_for_low_memory_callback(PalNotificationObject* callback) { + // No error handling here - if this doesn't work, then we will just + // consume more memory. There's nothing sensible that we could do in + // error handling. We also leak both the low memory notification object + // handle and the wait object handle. We'll need them until the program + // exits, so there's little point doing anything else. + // + // We only try to register once. If this fails, give up. Even if we + // create multiple PAL objects, we don't want to get more than one + // callback. + if (!registered_for_notifications.exchange(true)) + { + lowMemoryObject = + CreateMemoryResourceNotification(LowMemoryResourceNotification); + HANDLE waitObject; + RegisterWaitForSingleObject( + &waitObject, + lowMemoryObject, + low_memory, + nullptr, + INFINITE, + WT_EXECUTEDEFAULT); + } + low_memory_callbacks.register_notification(callback); } From d85c969642d34e70b04135bf0e20b6645e6e51e7 Mon Sep 17 00:00:00 2001 From: Nathaniel Filardo Date: Mon, 7 Sep 2020 10:42:10 +0100 Subject: [PATCH 5/8] fully-static PALs --- src/pal/pal_bsd.h | 2 +- src/pal/pal_bsd_aligned.h | 2 +- src/pal/pal_concept.h | 31 ++++++++++++++----------------- src/pal/pal_freebsd_kernel.h | 8 ++++---- src/pal/pal_haiku.h | 2 +- src/pal/pal_linux.h | 2 +- src/pal/pal_open_enclave.h | 2 +- src/pal/pal_plain.h | 4 ++-- src/pal/pal_posix.h | 10 +++++----- src/pal/pal_windows.h | 16 ++++++++-------- 10 files changed, 38 insertions(+), 41 deletions(-) diff --git a/src/pal/pal_bsd.h b/src/pal/pal_bsd.h index c446dcbc1..c4086190e 100644 --- a/src/pal/pal_bsd.h +++ b/src/pal/pal_bsd.h @@ -31,7 +31,7 @@ namespace snmalloc * operating system to replace the pages with CoW copies of a zero page at * any point between the call and the next write to that page. */ - void notify_not_using(void* p, size_t size) noexcept + static void notify_not_using(void* p, size_t size) noexcept { SNMALLOC_ASSERT(is_aligned_block(p, size)); madvise(p, size, MADV_FREE); diff --git a/src/pal/pal_bsd_aligned.h b/src/pal/pal_bsd_aligned.h index d5d57661b..7acefb710 100644 --- a/src/pal/pal_bsd_aligned.h +++ b/src/pal/pal_bsd_aligned.h @@ -29,7 +29,7 @@ namespace snmalloc * Reserve memory at a specific alignment. */ template - void* reserve_aligned(size_t size) noexcept + static void* reserve_aligned(size_t size) noexcept { // Alignment must be a power of 2. SNMALLOC_ASSERT(size == bits::next_pow2(size)); diff --git a/src/pal/pal_concept.h b/src/pal/pal_concept.h index 33f54f4cd..0407c856e 100644 --- a/src/pal/pal_concept.h +++ b/src/pal/pal_concept.h @@ -35,20 +35,17 @@ namespace snmalloc * PALs expose a basic library of memory operations. */ template - concept ConceptPAL_memops = requires(PAL p, void* vp, size_t sz) + concept ConceptPAL_memops = requires(void* vp, size_t sz) { - { p.notify_not_using(vp, sz) } noexcept -> ConceptSame; + { PAL::notify_not_using(vp, sz) } noexcept -> ConceptSame; - /* For reasons unknown, these seem to tickle some bug in MSVC */ -# if !defined(_MSC_VER) - { p.template notify_using(vp, sz) } noexcept + { PAL::template notify_using(vp, sz) } noexcept -> ConceptSame; - { p.template notify_using(vp, sz) } noexcept + { PAL::template notify_using(vp, sz) } noexcept -> ConceptSame; -# endif - { p.template zero(vp, sz) } noexcept -> ConceptSame; - { p.template zero(vp, sz) } noexcept -> ConceptSame; + { PAL::template zero(vp, sz) } noexcept -> ConceptSame; + { PAL::template zero(vp, sz) } noexcept -> ConceptSame; }; /** @@ -57,7 +54,7 @@ namespace snmalloc template concept ConceptPAL_reserve_at_least = requires(PAL p, void* vp, size_t sz) { - { p.reserve_at_least(sz) } noexcept + { PAL::reserve_at_least(sz) } noexcept -> ConceptSame>; }; @@ -65,21 +62,21 @@ namespace snmalloc * Some PALs expose a richer allocator which understands aligned allocations */ template - concept ConceptPAL_reserve_aligned = requires(PAL p, size_t sz) + concept ConceptPAL_reserve_aligned = requires(size_t sz) { - { p.template reserve_aligned(sz) } noexcept -> ConceptSame; - { p.template reserve_aligned(sz) } noexcept -> ConceptSame; + { PAL::template reserve_aligned(sz) } noexcept -> ConceptSame; + { PAL::template reserve_aligned(sz) } noexcept + -> ConceptSame; }; /** * Some PALs can provide memory pressure callbacks. */ template - concept ConceptPAL_mem_low_notify = - requires(PAL p, PalNotificationObject* pno) + concept ConceptPAL_mem_low_notify = requires(PalNotificationObject* pno) { - { p.expensive_low_memory_check() } -> ConceptSame; - { p.register_for_low_memory_callback(pno) } -> ConceptSame; + { PAL::expensive_low_memory_check() } -> ConceptSame; + { PAL::register_for_low_memory_callback(pno) } -> ConceptSame; }; /** diff --git a/src/pal/pal_freebsd_kernel.h b/src/pal/pal_freebsd_kernel.h index 826c1dc2a..7bab485e8 100644 --- a/src/pal/pal_freebsd_kernel.h +++ b/src/pal/pal_freebsd_kernel.h @@ -34,7 +34,7 @@ namespace snmalloc } /// Notify platform that we will not be using these pages - void notify_not_using(void* p, size_t size) + static void notify_not_using(void* p, size_t size) { vm_offset_t addr = get_vm_offset(p); kmem_unback(kernel_object, addr, size); @@ -42,7 +42,7 @@ namespace snmalloc /// Notify platform that we will be using these pages template - void notify_using(void* p, size_t size) + static void notify_using(void* p, size_t size) { vm_offset_t addr = get_vm_offset(p); int flags = M_WAITOK | ((zero_mem == YesZero) ? M_ZERO : 0); @@ -54,13 +54,13 @@ namespace snmalloc /// OS specific function for zeroing memory template - void zero(void* p, size_t size) + static void zero(void* p, size_t size) { ::bzero(p, size); } template - void* reserve_aligned(size_t size) noexcept + static void* reserve_aligned(size_t size) noexcept { SNMALLOC_ASSERT(size == bits::next_pow2(size)); SNMALLOC_ASSERT(size >= minimum_alloc_size); diff --git a/src/pal/pal_haiku.h b/src/pal/pal_haiku.h index 05ba71a06..1b7cc1e8a 100644 --- a/src/pal/pal_haiku.h +++ b/src/pal/pal_haiku.h @@ -31,7 +31,7 @@ namespace snmalloc * Notify platform that we will not be needing these pages. * Haiku does not provide madvise call per say only the posix equivalent. */ - void notify_not_using(void* p, size_t size) noexcept + static void notify_not_using(void* p, size_t size) noexcept { SNMALLOC_ASSERT(is_aligned_block(p, size)); posix_madvise(p, size, POSIX_MADV_DONTNEED); diff --git a/src/pal/pal_linux.h b/src/pal/pal_linux.h index a645c163c..74999afc3 100644 --- a/src/pal/pal_linux.h +++ b/src/pal/pal_linux.h @@ -37,7 +37,7 @@ namespace snmalloc * clear the underlying memory range. */ template - void zero(void* p, size_t size) noexcept + static void zero(void* p, size_t size) noexcept { // QEMU does not seem to be giving the desired behaviour for // MADV_DONTNEED. switch back to memset only for QEMU. diff --git a/src/pal/pal_open_enclave.h b/src/pal/pal_open_enclave.h index 0183c9f9c..420c49d57 100644 --- a/src/pal/pal_open_enclave.h +++ b/src/pal/pal_open_enclave.h @@ -62,7 +62,7 @@ namespace snmalloc } template - void zero(void* p, size_t size) noexcept + static void zero(void* p, size_t size) noexcept { oe_memset_s(p, size, 0, size); } diff --git a/src/pal/pal_plain.h b/src/pal/pal_plain.h index a7cd19990..9d8c8d9ec 100644 --- a/src/pal/pal_plain.h +++ b/src/pal/pal_plain.h @@ -11,11 +11,11 @@ namespace snmalloc { public: // Notify platform that we will not be using these pages - void notify_not_using(void*, size_t) noexcept {} + static void notify_not_using(void*, size_t) noexcept {} // Notify platform that we will not be using these pages template - void notify_using(void* p, size_t size) noexcept + static void notify_using(void* p, size_t size) noexcept { if constexpr (zero_mem == YesZero) { diff --git a/src/pal/pal_posix.h b/src/pal/pal_posix.h index 03d39ccce..6fbecad3c 100644 --- a/src/pal/pal_posix.h +++ b/src/pal/pal_posix.h @@ -131,7 +131,7 @@ namespace snmalloc * high memory pressure conditions, though on Linux this seems to impose * too much of a performance penalty. */ - void notify_not_using(void* p, size_t size) noexcept + static void notify_not_using(void* p, size_t size) noexcept { SNMALLOC_ASSERT(is_aligned_block(p, size)); #ifdef USE_POSIX_COMMIT_CHECKS @@ -150,7 +150,7 @@ namespace snmalloc * function. */ template - void notify_using(void* p, size_t size) noexcept + static void notify_using(void* p, size_t size) noexcept { SNMALLOC_ASSERT( is_aligned_block(p, size) || (zero_mem == NoZero)); @@ -163,7 +163,7 @@ namespace snmalloc #endif if constexpr (zero_mem == YesZero) - static_cast(this)->template zero(p, size); + zero(p, size); } /** @@ -178,7 +178,7 @@ namespace snmalloc * calling bzero at some point. */ template - void zero(void* p, size_t size) noexcept + static void zero(void* p, size_t size) noexcept { if (page_aligned || is_aligned_block(p, size)) { @@ -207,7 +207,7 @@ namespace snmalloc * POSIX does not define a portable interface for specifying alignment * greater than a page. */ - std::pair reserve_at_least(size_t size) noexcept + static std::pair reserve_at_least(size_t size) noexcept { SNMALLOC_ASSERT(size == bits::next_pow2(size)); diff --git a/src/pal/pal_windows.h b/src/pal/pal_windows.h index 1e6215ff6..39e055603 100644 --- a/src/pal/pal_windows.h +++ b/src/pal/pal_windows.h @@ -64,7 +64,7 @@ namespace snmalloc * Check whether the low memory state is still in effect. This is an * expensive operation and should not be on any fast paths. */ - bool expensive_low_memory_check() + static bool expensive_low_memory_check() { BOOL result; QueryMemoryResourceNotification(lowMemoryObject, &result); @@ -113,7 +113,7 @@ namespace snmalloc } /// Notify platform that we will not be using these pages - void notify_not_using(void* p, size_t size) noexcept + static void notify_not_using(void* p, size_t size) noexcept { SNMALLOC_ASSERT(is_aligned_block(p, size)); @@ -125,7 +125,7 @@ namespace snmalloc /// Notify platform that we will be using these pages template - void notify_using(void* p, size_t size) noexcept + static void notify_using(void* p, size_t size) noexcept { SNMALLOC_ASSERT( is_aligned_block(p, size) || (zero_mem == NoZero)); @@ -138,7 +138,7 @@ namespace snmalloc /// OS specific function for zeroing memory template - void zero(void* p, size_t size) noexcept + static void zero(void* p, size_t size) noexcept { if (page_aligned || is_aligned_block(p, size)) { @@ -151,13 +151,13 @@ namespace snmalloc } # ifdef USE_SYSTEMATIC_TESTING - size_t& systematic_bump_ptr() + static size_t& systematic_bump_ptr() { static size_t bump_ptr = (size_t)0x4000'0000'0000; return bump_ptr; } - std::pair reserve_at_least(size_t size) noexcept + static std::pair reserve_at_least(size_t size) noexcept { // Magic number for over-allocating chosen by the Pal // These should be further refined based on experiments. @@ -183,7 +183,7 @@ namespace snmalloc } # elif defined(PLATFORM_HAS_VIRTUALALLOC2) template - void* reserve_aligned(size_t size) noexcept + static void* reserve_aligned(size_t size) noexcept { SNMALLOC_ASSERT(size == bits::next_pow2(size)); SNMALLOC_ASSERT(size >= minimum_alloc_size); @@ -213,7 +213,7 @@ namespace snmalloc return ret; } # else - std::pair reserve_at_least(size_t size) noexcept + static std::pair reserve_at_least(size_t size) noexcept { SNMALLOC_ASSERT(size == bits::next_pow2(size)); From 4bddb731c845751a3baf79524accfb6375200d06 Mon Sep 17 00:00:00 2001 From: Nathaniel Filardo Date: Mon, 7 Sep 2020 11:23:28 +0100 Subject: [PATCH 6/8] AddressSpaceManager is not a PAL --- src/mem/address_space.h | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/mem/address_space.h b/src/mem/address_space.h index 00dc21b4f..31b436d38 100644 --- a/src/mem/address_space.h +++ b/src/mem/address_space.h @@ -14,7 +14,7 @@ namespace snmalloc * usual complexity of a buddy allocator. */ template - class AddressSpaceManager : public PAL + class AddressSpaceManager { /** * Stores the blocks of address space @@ -181,8 +181,7 @@ namespace snmalloc if constexpr (pal_supports) { if (size >= PAL::minimum_alloc_size) - return static_cast(this)->template reserve_aligned( - size); + return PAL::template reserve_aligned(size); } void* res; @@ -197,8 +196,7 @@ namespace snmalloc if constexpr (pal_supports) { block_size = PAL::minimum_alloc_size; - block = static_cast(this)->template reserve_aligned( - block_size); + block = PAL::template reserve_aligned(block_size); } else { @@ -206,8 +204,7 @@ namespace snmalloc // Hold lock here as a race could cause additional requests to // the PAL, and this could lead to suprious OOM. This is // particularly bad if the PAL gives all the memory on first call. - auto block_and_size = - static_cast(this)->reserve_at_least(size * 2); + auto block_and_size = PAL::reserve_at_least(size * 2); block = block_and_size.first; block_size = block_and_size.second; From 0bf621185abff1f826908d926e843c4c2d47b8c8 Mon Sep 17 00:00:00 2001 From: Nathaniel Filardo Date: Mon, 7 Sep 2020 11:57:38 +0100 Subject: [PATCH 7/8] MemoryProviderStateMixin is not a PAL --- src/mem/alloc.h | 12 ++++++------ src/mem/largealloc.h | 16 +++++++++------- src/mem/mediumslab.h | 6 +++--- src/mem/slab.h | 13 +++++-------- src/test/perf/low_memory/low-memory.cc | 6 +++--- 5 files changed, 26 insertions(+), 27 deletions(-) diff --git a/src/mem/alloc.h b/src/mem/alloc.h index 74380d1d7..fac0f1bd8 100644 --- a/src/mem/alloc.h +++ b/src/mem/alloc.h @@ -1063,7 +1063,7 @@ namespace snmalloc void* p = remove_cache_friendly_offset(head, sizeclass); if constexpr (zero_mem == YesZero) { - large_allocator.memory_provider.zero(p, sizeclass_to_size(sizeclass)); + MemoryProvider::Pal::zero(p, sizeclass_to_size(sizeclass)); } return p; } @@ -1109,8 +1109,8 @@ namespace snmalloc SlabLink* link = sl.get_next(); slab = get_slab(link); auto& ffl = small_fast_free_lists[sizeclass]; - return slab->alloc( - sl, ffl, rsize, large_allocator.memory_provider); + return slab->alloc( + sl, ffl, rsize); } return small_alloc_rare(sizeclass, size); } @@ -1182,7 +1182,7 @@ namespace snmalloc if constexpr (zero_mem == YesZero) { - large_allocator.memory_provider.zero(p, sizeclass_to_size(sizeclass)); + MemoryProvider::Pal::zero(p, sizeclass_to_size(sizeclass)); } return p; } @@ -1313,7 +1313,7 @@ namespace snmalloc if (slab != nullptr) { - p = slab->alloc(size, large_allocator.memory_provider); + p = slab->alloc(size); if (slab->full()) sc->pop(); @@ -1336,7 +1336,7 @@ namespace snmalloc slab->init(public_state(), sizeclass, rsize); chunkmap().set_slab(slab); - p = slab->alloc(size, large_allocator.memory_provider); + p = slab->alloc(size); if (!slab->full()) sc->insert(slab); diff --git a/src/mem/largealloc.h b/src/mem/largealloc.h index ff85bb56c..34f5ce7e7 100644 --- a/src/mem/largealloc.h +++ b/src/mem/largealloc.h @@ -57,7 +57,7 @@ namespace snmalloc // global state of the allocator. This is currently stored in the memory // provider, so we add this in. template - class MemoryProviderStateMixin : public PalNotificationObject, public PAL + class MemoryProviderStateMixin : public PalNotificationObject { /** * Simple flag for checking if another instance of lazy-decommit is @@ -76,6 +76,8 @@ namespace snmalloc std::atomic peak_memory_used_bytes{0}; public: + using Pal = PAL; + /** * Memory current available in large_stacks */ @@ -258,7 +260,7 @@ namespace snmalloc p = memory_provider.template reserve(large_class); if (p == nullptr) return nullptr; - memory_provider.template notify_using(p, rsize); + MemoryProvider::Pal::template notify_using(p, rsize); } else { @@ -276,19 +278,19 @@ namespace snmalloc // The first page is already in "use" for the stack element, // this will need zeroing for a YesZero call. if constexpr (zero_mem == YesZero) - memory_provider.template zero(p, OS_PAGE_SIZE); + MemoryProvider::Pal::template zero(p, OS_PAGE_SIZE); // Notify we are using the rest of the allocation. // Passing zero_mem ensures the PAL provides zeroed pages if // required. - memory_provider.template notify_using( + MemoryProvider::Pal::template notify_using( pointer_offset(p, OS_PAGE_SIZE), rsize - OS_PAGE_SIZE); } else { // This is a superslab that has not been decommitted. if constexpr (zero_mem == YesZero) - memory_provider.template zero( + MemoryProvider::Pal::template zero( p, bits::align_up(size, OS_PAGE_SIZE)); else UNUSED(size); @@ -304,7 +306,7 @@ namespace snmalloc if constexpr (decommit_strategy == DecommitSuperLazy) { static_assert( - pal_supports, + pal_supports, "A lazy decommit strategy cannot be implemented on platforms " "without low memory notifications"); } @@ -316,7 +318,7 @@ namespace snmalloc (decommit_strategy != DecommitNone) && (large_class != 0 || decommit_strategy == DecommitSuper)) { - memory_provider.notify_not_using( + MemoryProvider::Pal::notify_not_using( pointer_offset(p, OS_PAGE_SIZE), rsize - OS_PAGE_SIZE); } diff --git a/src/mem/mediumslab.h b/src/mem/mediumslab.h index 59a10f2e0..9dcfa6651 100644 --- a/src/mem/mediumslab.h +++ b/src/mem/mediumslab.h @@ -76,8 +76,8 @@ namespace snmalloc return sizeclass; } - template - void* alloc(size_t size, MemoryProvider& memory_provider) + template + void* alloc(size_t size) { SNMALLOC_ASSERT(!full()); @@ -86,7 +86,7 @@ namespace snmalloc free--; if constexpr (zero_mem == YesZero) - memory_provider.zero(p, size); + PAL::zero(p, size); else UNUSED(size); diff --git a/src/mem/slab.h b/src/mem/slab.h index 8c14fe364..c59e82e62 100644 --- a/src/mem/slab.h +++ b/src/mem/slab.h @@ -36,12 +36,9 @@ namespace snmalloc * Returns the link as the allocation, and places the free list into the * `fast_free_list` for further allocations. */ - template - SNMALLOC_FAST_PATH void* alloc( - SlabList& sl, - FreeListHead& fast_free_list, - size_t rsize, - MemoryProvider& memory_provider) + template + SNMALLOC_FAST_PATH void* + alloc(SlabList& sl, FreeListHead& fast_free_list, size_t rsize) { // Read the head from the metadata stored in the superslab. Metaslab& meta = get_meta(); @@ -73,9 +70,9 @@ namespace snmalloc if constexpr (zero_mem == YesZero) { if (rsize < PAGE_ALIGNED_SIZE) - memory_provider.zero(p, rsize); + PAL::zero(p, rsize); else - memory_provider.template zero(p, rsize); + PAL::template zero(p, rsize); } else { diff --git a/src/test/perf/low_memory/low-memory.cc b/src/test/perf/low_memory/low-memory.cc index 68007b4a7..83d8742d1 100644 --- a/src/test/perf/low_memory/low-memory.cc +++ b/src/test/perf/low_memory/low-memory.cc @@ -98,17 +98,17 @@ void reduce_pressure(Queue& allocations) * Template parameter required to handle `if constexpr` always evaluating both * sides. */ -template +template void register_for_pal_notifications() { - PAL::register_for_low_memory_callback(&update_epoch); + MemoryProvider::Pal::register_for_low_memory_callback(&update_epoch); } int main(int argc, char** argv) { opt::Opt opt(argc, argv); - if constexpr (pal_supports) + if constexpr (pal_supports) { register_for_pal_notifications(); } From c555abb86b2c8f27c28a0d12adc34c17317da2d3 Mon Sep 17 00:00:00 2001 From: Nathaniel Filardo Date: Wed, 9 Sep 2020 10:12:29 +0100 Subject: [PATCH 8/8] Update README.md PAL discussion w/ sprinkled "static" --- README.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 5f69911f2..84a0db27e 100644 --- a/README.md +++ b/README.md @@ -160,12 +160,12 @@ your system. The PAL must implement the following methods: ```c++ -[[noreturn]] void error(const char* const str) noexcept; +[[noreturn]] static void error(const char* const str) noexcept; ``` Report a fatal error and exit. ```c++ -void notify_not_using(void* p, size_t size) noexcept; +static void notify_not_using(void* p, size_t size) noexcept; ``` Notify the system that the range of memory from `p` to `p` + `size` is no longer in use, allowing the underlying physical pages to recycled for other @@ -173,7 +173,7 @@ purposes. ```c++ template -void notify_using(void* p, size_t size) noexcept; +static void notify_using(void* p, size_t size) noexcept; ``` Notify the system that the range of memory from `p` to `p` + `size` is now in use. On systems that lazily provide physical memory to virtual mappings, this @@ -183,7 +183,7 @@ responsible for ensuring that the newly requested memory is full of zeros. ```c++ template -void zero(void* p, size_t size) noexcept; +static void zero(void* p, size_t size) noexcept; ``` Zero the range of memory from `p` to `p` + `size`. This may be a simple `memset` call, but the `page_aligned` template parameter @@ -194,8 +194,8 @@ pages, rather than zeroing them synchronously in this call ```c++ template -void* reserve_aligned(size_t size) noexcept; -std::pair reserve_at_least(size_t size) noexcept; +static void* reserve_aligned(size_t size) noexcept; +static std::pair reserve_at_least(size_t size) noexcept; ``` Only one of these needs to be implemented, depending on whether the underlying system can provide strongly aligned memory regions.