From 4ed15def79ad397a84c287b1deab83b8aa138ae9 Mon Sep 17 00:00:00 2001 From: David Chisnall Date: Thu, 1 Aug 2019 10:44:54 +0100 Subject: [PATCH] [NFC] Remove some code duplication in the PALs. Pull out a generic POSIX PAL as a superclass for the Linux and generic BSD PALs. Now we have FreeBSD and Linux adding OS-specific behaviour, OpenBSD as a named subclass of the generic BSD PAL that doesn't add any behaviour. I believe a NetBSD PAL should now be identical to the OpenBSD one - patches welcome if anyone wants to test one! --- src/pal/pal_bsd.h | 85 +++++++--------------------- src/pal/pal_freebsd.h | 14 +++-- src/pal/pal_linux.h | 67 ++++++---------------- src/pal/pal_openbsd.h | 17 +++++- src/pal/pal_posix.h | 125 ++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 185 insertions(+), 123 deletions(-) create mode 100644 src/pal/pal_posix.h diff --git a/src/pal/pal_bsd.h b/src/pal/pal_bsd.h index cc996c2..25ba3bd 100644 --- a/src/pal/pal_bsd.h +++ b/src/pal/pal_bsd.h @@ -3,6 +3,7 @@ #if (defined(__FreeBSD__) || defined(__OpenBSD__)) && !defined(_KERNEL) # include "../ds/bits.h" # include "../mem/allocconfig.h" +# include "pal_posix.h" # include # include @@ -10,84 +11,36 @@ namespace snmalloc { - class PALBSD + /** + * Generic *BSD PAL mixin. This provides features that are common to the BSD + * family. + */ + template + class PALBSD : public PALPOSIX { public: /** * Bitmap of PalFeatures flags indicating the optional features that this * PAL supports. + * + * The generic BSD PAL does not add any features that are not supported by + * generic POSIX systems, but explicitly declares this variable to remind + * anyone who extends this class that they may need to modify this field. */ - static constexpr uint64_t pal_features = 0; - static void error(const char* const str) - { - puts(str); - abort(); - } + static constexpr uint64_t pal_features = PALPOSIX::pal_features; - /// Notify platform that we will not be using these pages + /** + * Notify platform that we will not be using these pages. + * + * BSD systems provide the `MADV_FREE` flag to `madvise`, which allows the + * 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 { assert(bits::is_aligned_block(p, size)); madvise(p, size, MADV_FREE); } - - /// Notify platform that we will be using these pages - template - void notify_using(void* p, size_t size) noexcept - { - assert( - bits::is_aligned_block(p, size) || (zero_mem == NoZero)); - if constexpr (zero_mem == YesZero) - { - zero(p, size); - } - else - { - UNUSED(size); - UNUSED(p); - } - } - - /// OS specific function for zeroing memory - template - void zero(void* p, size_t size) noexcept - { - if (page_aligned || bits::is_aligned_block(p, size)) - { - assert(bits::is_aligned_block(p, size)); - void* r = mmap( - p, - size, - PROT_READ | PROT_WRITE, - MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, - -1, - 0); - - if (r != MAP_FAILED) - return; - } - - bzero(p, size); - } - - template - void* reserve(const size_t* size) noexcept - { - size_t request = *size; - - void* p = mmap( - nullptr, - request, - PROT_READ | PROT_WRITE, - MAP_PRIVATE | MAP_ANONYMOUS, - -1, - 0); - - if (p == MAP_FAILED) - error("Out of memory"); - - return p; - } }; } // namespace snmalloc #endif diff --git a/src/pal/pal_freebsd.h b/src/pal/pal_freebsd.h index 2924fa4..b853281 100644 --- a/src/pal/pal_freebsd.h +++ b/src/pal/pal_freebsd.h @@ -11,19 +11,25 @@ namespace snmalloc { - class PALFBSD : public PALBSD + class PALFBSD : public PALBSD { public: /** * Bitmap of PalFeatures flags indicating the optional features that this * PAL supports. */ - static constexpr uint64_t pal_features = AlignedAllocation | LazyCommit; + static constexpr uint64_t pal_features = + AlignedAllocation | PALBSD::pal_features; + /** + * Reserve memory. + * + * FreeBSD supports requesting pages at an arbitrary alignment, which + * improves efficiency of snmalloc. + */ template void* reserve(const size_t* size, size_t align) noexcept { - size_t request = *size; // Alignment must be a power of 2. assert(align == bits::next_pow2(align)); @@ -33,7 +39,7 @@ namespace snmalloc void* p = mmap( nullptr, - request, + *size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS | MAP_ALIGNED(log2align), -1, diff --git a/src/pal/pal_linux.h b/src/pal/pal_linux.h index dcb75dd..e159831 100644 --- a/src/pal/pal_linux.h +++ b/src/pal/pal_linux.h @@ -3,6 +3,7 @@ #if defined(__linux__) # include "../ds/bits.h" # include "../mem/allocconfig.h" +# include "pal_posix.h" # include # include @@ -11,47 +12,28 @@ extern "C" int puts(const char* str); namespace snmalloc { - class PALLinux + class PALLinux : public PALPOSIX { public: /** * Bitmap of PalFeatures flags indicating the optional features that this * PAL supports. + * + * Linux does not support any features other than those in a generic POSIX + * platform. This field is declared explicitly to remind anyone who + * extends this PAL that they may need to extend the set of advertised + * features. */ - static constexpr uint64_t pal_features = LazyCommit; - static void error(const char* const str) - { - puts(str); - abort(); - } + static constexpr uint64_t pal_features = PALPOSIX::pal_features; - /// Notify platform that we will not be using these pages - void notify_not_using(void* p, size_t size) noexcept - { - assert(bits::is_aligned_block(p, size)); - // Do nothing. Don't call madvise here, as the system call slows the - // allocator down too much. - UNUSED(p); - UNUSED(size); - } - - /// Notify platform that we will be using these pages - template - void notify_using(void* p, size_t size) noexcept - { - assert( - bits::is_aligned_block(p, size) || (zero_mem == NoZero)); - - if constexpr (zero_mem == YesZero) - zero(p, size); - else - { - UNUSED(p); - UNUSED(size); - } - } - - /// OS specific function for zeroing memory + /** + * OS specific function for zeroing memory. + * + * Linux implements an unusual interpretation of `MADV_DONTNEED`, which + * immediately resets the pages to the zero state (rather than marking them + * as sensible ones to swap out in high memory pressure). We use this to + * clear the underlying memory range. + */ template void zero(void* p, size_t size) noexcept { @@ -65,23 +47,6 @@ namespace snmalloc ::memset(p, 0, size); } } - - template - void* reserve(const size_t* size) noexcept - { - void* p = mmap( - nullptr, - *size, - PROT_READ | PROT_WRITE, - MAP_PRIVATE | MAP_ANONYMOUS, - -1, - 0); - - if (p == MAP_FAILED) - error("Out of memory"); - - return p; - } }; } // namespace snmalloc #endif diff --git a/src/pal/pal_openbsd.h b/src/pal/pal_openbsd.h index fb1dbc0..0daf4ca 100644 --- a/src/pal/pal_openbsd.h +++ b/src/pal/pal_openbsd.h @@ -11,10 +11,23 @@ namespace snmalloc { - class PALOBSD : public PALBSD + /** + * OpenBSD platform abstraction layer. + * + * OpenBSD behaves exactly like a generic BSD platform but this class exists + * as a place to add OpenBSD-specific behaviour later, if required. + */ + class PALOBSD : public PALBSD { public: - static constexpr uint64_t pal_features = LazyCommit; + /** + * The features exported by this PAL. + * + * Currently, these are identical to the generic BSD PAL. This field is + * declared explicitly to remind anyone who modifies this class that they + * should add any required features. + */ + static constexpr uint64_t pal_features = PALBSD; }; } // namespace snmalloc #endif diff --git a/src/pal/pal_posix.h b/src/pal/pal_posix.h new file mode 100644 index 0000000..4cf1e62 --- /dev/null +++ b/src/pal/pal_posix.h @@ -0,0 +1,125 @@ +#pragma once + +#include "../ds/bits.h" +#include "../mem/allocconfig.h" + +#include +#include + +extern "C" int puts(const char* str); + +namespace snmalloc +{ + template + class PALPOSIX + { + public: + /** + * Bitmap of PalFeatures flags indicating the optional features that this + * PAL supports. + * + * POSIX systems are assumed to support lazy commit. + */ + static constexpr uint64_t pal_features = LazyCommit; + static void error(const char* const str) noexcept + { + puts(str); + abort(); + } + + /** + * Notify platform that we will not be using these pages. + * + * This does nothing in a generic POSIX implementation. Most POSIX systems + * provide an `madvise` call that can be used to return pages to the OS in + * 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 + { + assert(bits::is_aligned_block(p, size)); + UNUSED(p); + UNUSED(size); + } + + /** + * Notify platform that we will be using these pages. + * + * On POSIX platforms, lazy commit means that this is a no-op, unless we + * are also zeroing the pages in which case we call the platform's `zero` + * function. + */ + template + void notify_using(void* p, size_t size) noexcept + { + assert( + bits::is_aligned_block(p, size) || (zero_mem == NoZero)); + + if constexpr (zero_mem == YesZero) + static_cast(this)->template zero(p, size); + else + { + UNUSED(p); + UNUSED(size); + } + } + + /** + * OS specific function for zeroing memory. + * + * The generic POSIX implementation uses mmap to map anonymous memory over + * the range for ranges larger than a page. The underlying OS is assumed + * to provide new CoW copies of the zero page. + * + * Note: On most systems it is faster for a single page to zero the memory + * explicitly than do this, we should probably tweak the threshold for + * calling bzero at some point. + */ + template + void zero(void* p, size_t size) noexcept + { + if (page_aligned || bits::is_aligned_block(p, size)) + { + assert(bits::is_aligned_block(p, size)); + void* r = mmap( + p, + size, + PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, + -1, + 0); + + if (r != MAP_FAILED) + return; + } + + bzero(p, size); + } + + /** + * Reserve memory. + * + * POSIX platforms support lazy commit, and so this also puts the memory in + * the lazy commit state (i.e. pages will be allocated on first use). + * + * POSIX does not define a portable interface for specifying alignment + * greater than a page. + */ + template + void* reserve(const size_t* size) noexcept + { + void* p = mmap( + nullptr, + *size, + PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS, + -1, + 0); + + if (p == MAP_FAILED) + OS::error("Out of memory"); + + return p; + } + }; +} // namespace snmalloc