CR Feedback

Removed stub from message queue, and use an actual allocation.
This commit is contained in:
Matthew Parkinson
2019-07-02 14:01:16 +01:00
parent fdcbcf7016
commit eb4e28e8d0
10 changed files with 111 additions and 96 deletions

View File

@@ -11,13 +11,9 @@
# define HEADER_GLOBAL __declspec(selectany)
# define likely(x) !!(x)
# define unlikely(x) !!(x)
# define SLOW_PATH NOINLINE
# define FAST_PATH ALWAYSINLINE
# ifdef NDEBUG
# define ASSUME(x)
# else
# define ASSUME(x) assert(x);
# endif
# define SNMALLOC_SLOW_PATH NOINLINE
# define SNMALLOC_FAST_PATH ALWAYSINLINE
# define SNMALLOC_PURE
#else
# define likely(x) __builtin_expect(!!(x), 1)
# define unlikely(x) __builtin_expect(!!(x), 0)
@@ -25,15 +21,9 @@
# include <emmintrin.h>
# define ALWAYSINLINE __attribute__((always_inline))
# define NOINLINE __attribute__((noinline))
# define SLOW_PATH NOINLINE
# define FAST_PATH ALWAYSINLINE
# ifdef NDEBUG
# define ASSUME(x) \
if (!(x)) \
__builtin_unreachable();
# else
# define ASSUME(x) assert(x);
# endif
# define SNMALLOC_SLOW_PATH NOINLINE
# define SNMALLOC_FAST_PATH ALWAYSINLINE
# define SNMALLOC_PURE __attribute__((const))
# ifdef __clang__
# define HEADER_GLOBAL __attribute__((selectany))
# else
@@ -68,13 +58,17 @@
#define UNUSED(x) ((void)(x))
#if __has_builtin(__builtin_assume)
# define SNMALLOC_ASSUME(x) __builtin_assume(x)
#ifndef NDEBUG
# define SNMALLOC_ASSUME(x) assert(x)
#else
# define SNMALLOC_ASSUME(x) \
do \
{ \
} while (0)
# if __has_builtin(__builtin_assume)
# define SNMALLOC_ASSUME(x) __builtin_assume((x))
# else
# define SNMALLOC_ASSUME(x) \
do \
{ \
} while (0)
# endif
#endif
// #define USE_LZCNT

View File

@@ -4,7 +4,6 @@
#include "helpers.h"
#include <utility>
namespace snmalloc
{
template<class T>
@@ -15,8 +14,8 @@ namespace snmalloc
std::is_same<decltype(T::next), std::atomic<T*>>::value,
"T->next must be a std::atomic<T*>");
std::atomic<T*> back;
T* front;
std::atomic<T*> back = nullptr;
T* front = nullptr;
public:
void invariant()

View File

@@ -337,7 +337,7 @@ namespace snmalloc
}
template<ZeroMem zero_mem = NoZero, AllowReserve allow_reserve = YesReserve>
SLOW_PATH ALLOCATOR void* alloc_not_small(size_t size)
SNMALLOC_SLOW_PATH ALLOCATOR void* alloc_not_small(size_t size)
{
handle_message_queue();
@@ -438,7 +438,7 @@ namespace snmalloc
#endif
}
FAST_PATH void dealloc(void* p)
SNMALLOC_FAST_PATH void dealloc(void* p)
{
#ifdef USE_MALLOC
return free(p);
@@ -470,7 +470,7 @@ namespace snmalloc
dealloc_not_small(p, size);
}
SLOW_PATH void dealloc_not_small(void* p, uint8_t size)
SNMALLOC_SLOW_PATH void dealloc_not_small(void* p, uint8_t size)
{
handle_message_queue();
@@ -649,7 +649,7 @@ namespace snmalloc
return (id >> (initial_shift + (r * REMOTE_SLOT_BITS))) & REMOTE_MASK;
}
FAST_PATH void
SNMALLOC_FAST_PATH void
dealloc(alloc_id_t target_id, void* p, sizeclass_t sizeclass)
{
this->size += sizeclass_to_size(sizeclass);
@@ -726,7 +726,6 @@ namespace snmalloc
DLList<Superslab> super_only_short_available;
RemoteCache remote;
Remote stub;
std::conditional_t<IsQueueInline, RemoteAllocator, RemoteAllocator*>
remote_alloc;
@@ -836,52 +835,62 @@ namespace snmalloc
void init_message_queue()
{
message_queue().init(&stub);
// Manufacture an allocation to prime the queue
// Using an actual allocation removes a conditional of a critical path.
message_queue().init(
reinterpret_cast<Remote*>(alloc<YesZero>(MIN_ALLOC_SIZE)));
}
FAST_PATH void handle_dealloc_remote(Remote* p)
SNMALLOC_FAST_PATH void handle_dealloc_remote(Remote* p)
{
if (likely(p != &stub))
{
Superslab* super = Superslab::get(p);
Superslab* super = Superslab::get(p);
#ifdef CHECK_CLIENT
if (p->target_id() != super->get_allocator()->id())
error("Detected memory corruption. Potential use-after-free");
if (p->target_id() != super->get_allocator()->id())
error("Detected memory corruption. Potential use-after-free");
#endif
if (likely(super->get_kind() == Super))
if (likely(super->get_kind() == Super))
{
Slab* slab = Slab::get(p);
Metaslab& meta = super->get_meta(slab);
if (likely(p->target_id() == id()))
{
Slab* slab = Slab::get(p);
Metaslab& meta = super->get_meta(slab);
if (likely(p->target_id() == id()))
{
small_dealloc_offseted(super, p, meta.sizeclass);
}
else
{
// Queue for remote dealloc elsewhere.
remote.dealloc(p->target_id(), p, meta.sizeclass);
}
small_dealloc_offseted(super, p, meta.sizeclass);
return;
}
}
handle_dealloc_remote_slow(p);
}
SNMALLOC_SLOW_PATH void handle_dealloc_remote_slow(Remote* p)
{
Superslab* super = Superslab::get(p);
if (likely(super->get_kind() == Medium))
{
Mediumslab* slab = Mediumslab::get(p);
if (p->target_id() == id())
{
sizeclass_t sizeclass = slab->get_sizeclass();
void* start = remove_cache_friendly_offset(p, sizeclass);
medium_dealloc(slab, start, sizeclass);
}
else
{
Mediumslab* slab = Mediumslab::get(p);
if (p->target_id() == id())
{
sizeclass_t sizeclass = slab->get_sizeclass();
void* start = remove_cache_friendly_offset(p, sizeclass);
medium_dealloc(slab, start, sizeclass);
}
else
{
// Queue for remote dealloc elsewhere.
remote.dealloc(p->target_id(), p, slab->get_sizeclass());
}
// Queue for remote dealloc elsewhere.
remote.dealloc(p->target_id(), p, slab->get_sizeclass());
}
}
else
{
assert(likely(p->target_id() != id()));
Slab* slab = Slab::get(p);
Metaslab& meta = super->get_meta(slab);
// Queue for remote dealloc elsewhere.
remote.dealloc(p->target_id(), p, meta.sizeclass);
}
}
SLOW_PATH void handle_message_queue_inner()
SNMALLOC_SLOW_PATH void handle_message_queue_inner()
{
for (size_t i = 0; i < REMOTE_BATCH; i++)
{
@@ -901,7 +910,7 @@ namespace snmalloc
remote.post(id());
}
FAST_PATH void handle_message_queue()
SNMALLOC_FAST_PATH void handle_message_queue()
{
// Inline the empty check, but not necessarily the full queue handling.
if (likely(message_queue().is_empty()))
@@ -1016,7 +1025,7 @@ namespace snmalloc
zero_mem == YesZero ? "zeromem" : "nozeromem",
allow_reserve == NoReserve ? "noreserve" : "reserve"));
ASSUME(size <= SLAB_SIZE);
SNMALLOC_ASSUME(size <= SLAB_SIZE);
sizeclass_t sizeclass = size_to_sizeclass(size);
stats().sizeclass_alloc(sizeclass);
@@ -1040,7 +1049,7 @@ namespace snmalloc
}
template<ZeroMem zero_mem, AllowReserve allow_reserve>
SLOW_PATH void* small_alloc_slow(sizeclass_t sizeclass)
SNMALLOC_SLOW_PATH void* small_alloc_slow(sizeclass_t sizeclass)
{
handle_message_queue();
size_t rsize = sizeclass_to_size(sizeclass);
@@ -1067,7 +1076,7 @@ namespace snmalloc
sl, ffl, rsize, large_allocator.memory_provider);
}
FAST_PATH void
SNMALLOC_FAST_PATH void
small_dealloc(Superslab* super, void* p, sizeclass_t sizeclass)
{
#ifdef CHECK_CLIENT
@@ -1082,7 +1091,7 @@ namespace snmalloc
small_dealloc_offseted(super, offseted, sizeclass);
}
FAST_PATH void
SNMALLOC_FAST_PATH void
small_dealloc_offseted(Superslab* super, void* p, sizeclass_t sizeclass)
{
MEASURE_TIME(small_dealloc, 4, 16);
@@ -1095,7 +1104,7 @@ namespace snmalloc
small_dealloc_offseted_slow(super, p, sizeclass);
}
SLOW_PATH void small_dealloc_offseted_slow(
SNMALLOC_SLOW_PATH void small_dealloc_offseted_slow(
Superslab* super, void* p, sizeclass_t sizeclass)
{
bool was_full = super->is_full();
@@ -1300,7 +1309,8 @@ namespace snmalloc
large_allocator.dealloc(slab, large_class);
}
void remote_dealloc(RemoteAllocator* target, void* p, sizeclass_t sizeclass)
SNMALLOC_FAST_PATH void
remote_dealloc(RemoteAllocator* target, void* p, sizeclass_t sizeclass)
{
MEASURE_TIME(remote_dealloc, 4, 16);

View File

@@ -189,7 +189,7 @@ namespace snmalloc
* (over the lifetime of this process). If the underlying system does not
* support low memory notifications, this will return 0.
*/
FAST_PATH
SNMALLOC_FAST_PATH
uint64_t low_memory_epoch()
{
if constexpr (pal_supports<LowMemoryNotification>())
@@ -235,7 +235,7 @@ namespace snmalloc
}
}
FAST_PATH void lazy_decommit_if_needed()
SNMALLOC_FAST_PATH void lazy_decommit_if_needed()
{
#ifdef TEST_LAZY_DECOMMIT
static_assert(

View File

@@ -99,7 +99,7 @@ namespace snmalloc
/// Store next pointer in a block. In Debug using magic value to detect some
/// simple corruptions.
static void store_next(void* p, void* head)
static SNMALLOC_FAST_PATH void store_next(void* p, void* head)
{
#ifndef CHECK_CLIENT
*static_cast<void**>(p) = head;
@@ -111,7 +111,7 @@ namespace snmalloc
/// Accessor function for the next pointer in a block.
/// In Debug checks for simple corruptions.
static void* follow_next(void* node)
static SNMALLOC_FAST_PATH void* follow_next(void* node)
{
#ifdef CHECK_CLIENT
uintptr_t next = *static_cast<uintptr_t*>(node);
@@ -195,11 +195,13 @@ namespace snmalloc
// Block is not full
assert(SLAB_SIZE > accounted_for);
// Keep variable so it appears in debugger.
size_t length = debug_slab_acyclic_free_list(slab);
UNUSED(length);
// Walk bump-free-list-segment accounting for unused space
void* curr = pointer_offset(slab, head);
while ((reinterpret_cast<size_t>(curr) & 1) == 0)
while ((address_cast(curr) & 1) == 0)
{
// Check we are looking at a correctly aligned block
void* start = curr;

View File

@@ -105,12 +105,13 @@ namespace snmalloc
std::atomic<PagemapEntry*> top[TOPLEVEL_ENTRIES]; // = {nullptr};
template<bool create_addr>
FAST_PATH PagemapEntry*
SNMALLOC_FAST_PATH PagemapEntry*
get_node(std::atomic<PagemapEntry*>* e, bool& result)
{
// The page map nodes are all allocated directly from the OS zero
// initialised with a system call. We don't need any ordered to guarantee
// to see that correctly.
// to see that correctly. The only transistions are monotone and handled
// by the slow path.
PagemapEntry* value = e->load(std::memory_order_relaxed);
if (likely(value > LOCKED_ENTRY))
@@ -129,7 +130,7 @@ namespace snmalloc
}
}
SLOW_PATH PagemapEntry*
SNMALLOC_SLOW_PATH PagemapEntry*
get_node_slow(std::atomic<PagemapEntry*>* e, bool& result)
{
// The page map nodes are all allocated directly from the OS zero
@@ -163,7 +164,7 @@ namespace snmalloc
}
template<bool create_addr>
FAST_PATH std::pair<Leaf*, size_t>
SNMALLOC_FAST_PATH std::pair<Leaf*, size_t>
get_leaf_index(uintptr_t addr, bool& result)
{
#ifdef FreeBSD_KERNEL
@@ -207,7 +208,7 @@ namespace snmalloc
}
template<bool create_addr>
FAST_PATH std::atomic<T>* get_addr(uintptr_t p, bool& success)
SNMALLOC_FAST_PATH std::atomic<T>* get_addr(uintptr_t p, bool& success)
{
auto leaf_ix = get_leaf_index<create_addr>(p, success);
return &(leaf_ix.first->values[leaf_ix.second]);

View File

@@ -5,6 +5,7 @@
namespace snmalloc
{
// Both usings should compile
// We use size_t as it generates better code.
using sizeclass_t = size_t;
// using sizeclass_t = uint8_t;
@@ -22,8 +23,12 @@ namespace snmalloc
// Don't use sizeclasses that are not a multiple of the alignment.
// For example, 24 byte allocations can be
// problematic for some data due to alignment issues.
return static_cast<sizeclass_t>(
auto sc = static_cast<sizeclass_t>(
bits::to_exp_mant_const<INTERMEDIATE_BITS, MIN_ALLOC_BITS>(size));
assert(sc == static_cast<uint8_t>(sc));
return sc;
}
constexpr static inline size_t large_sizeclass_to_size(uint8_t large_class)
@@ -140,27 +145,28 @@ namespace snmalloc
}
#ifdef CACHE_FRIENDLY_OFFSET
inline static void*
SNMALLOC_FAST_PATH static void*
remove_cache_friendly_offset(void* p, sizeclass_t sizeclass)
{
size_t mask = sizeclass_to_inverse_cache_friendly_mask(sizeclass);
return p = (void*)((uintptr_t)p & mask);
}
inline static uint16_t
SNMALLOC_FAST_PATH static uint16_t
remove_cache_friendly_offset(uint16_t relative, sizeclass_t sizeclass)
{
size_t mask = sizeclass_to_inverse_cache_friendly_mask(sizeclass);
return relative & mask;
}
#else
inline static void*
SNMALLOC_FAST_PATH static void*
remove_cache_friendly_offset(void* p, sizeclass_t sizeclass)
{
UNUSED(sizeclass);
return p;
}
inline static uint16_t
SNMALLOC_FAST_PATH static uint16_t
remove_cache_friendly_offset(uint16_t relative, sizeclass_t sizeclass)
{
UNUSED(sizeclass);

View File

@@ -7,11 +7,14 @@ namespace snmalloc
{
constexpr size_t PTR_BITS = bits::next_pow2_bits_const(sizeof(void*));
constexpr static size_t sizeclass_lookup_index(size_t s)
constexpr static SNMALLOC_PURE size_t sizeclass_lookup_index(const size_t s)
{
// We shift by PTR_BITS as makes code-gen for the table lookup nicer.
// We could shift by MIN_ALLOC_BITS, but then there is a slightly more
// complex sequence.
// We subtract and shirt to reduce the size of the table, i.e. we don't have
// to store a value for every size class.
// We could shift by MIN_ALLOC_BITS, as this would give us the most
// compressed table, but by shifting by PTR_BITS the code-gen is better
// as the most important path using this subsequently shifts left by
// PTR_BITS, hence they can be fused into a single mask.
return (s - 1) >> PTR_BITS;
}
@@ -112,7 +115,7 @@ namespace snmalloc
if ((size - 1) <= (SLAB_SIZE - 1))
{
auto index = sizeclass_lookup_index(size);
ASSUME(index <= sizeclass_lookup_index(SLAB_SIZE));
SNMALLOC_ASSUME(index <= sizeclass_lookup_index(SLAB_SIZE));
return sizeclass_metadata.sizeclass_lookup[index];
}

View File

@@ -150,7 +150,7 @@ namespace snmalloc
// Returns true, if it deallocation can proceed without changing any status
// bits. Note that this does remove the use from the meta slab, so it
// doesn't need doing on the slow path.
FAST_PATH bool dealloc_fast(Superslab* super, void* p)
SNMALLOC_FAST_PATH bool dealloc_fast(Superslab* super, void* p)
{
Metaslab& meta = super->get_meta(this);
#ifdef CHECK_CLIENT
@@ -187,7 +187,7 @@ namespace snmalloc
// Returns a complex return code for managing the superslab meta data.
// i.e. This deallocation could make an entire superslab free.
template<typename MemoryProvider>
SLOW_PATH typename Superslab::Action dealloc_slow(
SNMALLOC_SLOW_PATH typename Superslab::Action dealloc_slow(
SlabList* sl, Superslab* super, void* p, MemoryProvider& memory_provider)
{
Metaslab& meta = super->get_meta(this);

View File

@@ -28,7 +28,7 @@ namespace snmalloc
class ThreadAllocUntypedWrapper
{
public:
static FAST_PATH Alloc*& get()
static SNMALLOC_FAST_PATH Alloc*& get()
{
return (Alloc*&)ThreadAllocUntyped::get();
}
@@ -76,7 +76,7 @@ namespace snmalloc
* The non-create case exists so that the `per_thread` variable can be a
* local static and not a global, allowing ODR to deduplicate it.
*/
static FAST_PATH Alloc*& get(bool create = true)
static SNMALLOC_FAST_PATH Alloc*& get(bool create = true)
{
static thread_local Alloc* per_thread;
if (!per_thread && create)
@@ -224,7 +224,7 @@ namespace snmalloc
* Private accessor to the per thread allocator
* Provides no checking for initialization
*/
static FAST_PATH Alloc*& inner_get()
static SNMALLOC_FAST_PATH Alloc*& inner_get()
{
static thread_local Alloc* per_thread;
return per_thread;
@@ -242,7 +242,7 @@ namespace snmalloc
/**
* Private initialiser for the per thread allocator
*/
static SLOW_PATH Alloc*& inner_init()
static SNMALLOC_SLOW_PATH Alloc*& inner_init()
{
Alloc*& per_thread = inner_get();
@@ -280,7 +280,7 @@ namespace snmalloc
* Public interface, returns the allocator for the current thread,
* constructing it if necessary.
*/
static FAST_PATH Alloc*& get()
static SNMALLOC_FAST_PATH Alloc*& get()
{
Alloc*& per_thread = inner_get();