From afc6283e01eb6189e47d1084ce3c1266ed7bc46c Mon Sep 17 00:00:00 2001 From: Matthew Parkinson Date: Wed, 24 Mar 2021 16:52:12 +0000 Subject: [PATCH] Threshold freelist wakeup When a slab has been fully allocated, then we no longer check it has entries until something returns an allocation to this slab. However, it is possible that only a single allocation is available, and then we can end up frequently on the slow path. This change only considers free lists that cover at least 1/8 of a slab. This means that we will hit the slow path less frequently. This also means that the randomisation changes will have more entropy: with a single element free list there is only one order. For large small sizes it can still be a single element, as 1/8 is of the slab capacity is below 1. We max out the trigger at 31 elements to reduce unneeded wasted space. --- src/mem/freelist.h | 50 +++++++++++++++++++++++++-------- src/mem/metaslab.h | 68 +++++++++++++++++++++++++++++---------------- src/mem/slab.h | 8 ++++-- src/mem/superslab.h | 22 +++------------ 4 files changed, 92 insertions(+), 56 deletions(-) diff --git a/src/mem/freelist.h b/src/mem/freelist.h index c19bbda..c0770d7 100644 --- a/src/mem/freelist.h +++ b/src/mem/freelist.h @@ -47,12 +47,12 @@ namespace snmalloc #endif } - static inline bool different_slab(uintptr_t p1, uintptr_t p2) + static inline bool different_slab(address_t p1, address_t p2) { return ((p1 ^ p2) >= SLAB_SIZE); } - static inline bool different_slab(uintptr_t p1, void* p2) + static inline bool different_slab(address_t p1, void* p2) { return different_slab(p1, address_cast(p2)); } @@ -78,16 +78,14 @@ namespace snmalloc // Simple involutional encoding. The bottom half of each word is // multiplied by a function of both global and local keys (the latter, // in practice, being the address of the previous list entry) and the - // resulting word's top half is XORed into the pointer value before it + // resulting word's top part is XORed into the pointer value before it // is stored. auto next = address_cast(next_object); - constexpr uintptr_t MASK = bits::one_at_bit(PRESERVE_BOTTOM_BITS) - 1; + constexpr address_t MASK = bits::one_at_bit(PRESERVE_BOTTOM_BITS) - 1; // Mix in local_key - // We shift local key to the critical bits have more effect on the high - // bits. - address_t lk = local_key; - auto key = (lk << PRESERVE_BOTTOM_BITS) + global_key; - next ^= (((next & MASK) + 1) * key) & ~MASK; + address_t key = (local_key + 1) * global_key; + next ^= (((next & MASK) + 1) * key) & + ~(bits::one_at_bit(PRESERVE_BOTTOM_BITS) - 1); next_object = reinterpret_cast(next); } #else @@ -143,7 +141,7 @@ namespace snmalloc { FreeObject* curr = nullptr; #ifdef CHECK_CLIENT - uintptr_t prev = 0; + address_t prev = 0; #endif uint16_t get_prev() @@ -310,7 +308,7 @@ namespace snmalloc */ void open(void* p) { - interleave = 0xDEADBEEF; + interleave = 0xDEADBEEF; // TODO RANDOM SNMALLOC_ASSERT(empty()); #ifdef CHECK_CLIENT @@ -323,6 +321,8 @@ namespace snmalloc #endif end[0] = &head[0]; end[1] = &head[1]; + + SNMALLOC_ASSERT(debug_length() == 0); } /** @@ -352,6 +352,34 @@ namespace snmalloc #endif } + /** + * Calculates the length of the queue. + * This is O(n) as it walks the queue. + * If this is needed in a non-debug setting then + * we should look at redesigning the queue. + */ + size_t debug_length() + { + size_t count = 0; + for (size_t i = 0; i < 2; i++) + { + uint16_t local_prev = HEAD_KEY; + EncodeFreeObjectReference* iter = &head[i]; + FreeObject* prev_obj = iter->read(local_prev); + uint16_t local_curr = initial_key(prev_obj) & 0xffff; + while (end[i] != iter) + { + FreeObject* next = iter->read(local_prev); + check_client(!different_slab(next, prev_obj), "Heap corruption"); + local_prev = local_curr; + local_curr = address_cast(next) & 0xffff; + count++; + iter = &next->next_object; + } + } + return count; + } + /** * Adds a terminator at the end of a free list, * but does not close the builder. Thus new elements diff --git a/src/mem/metaslab.h b/src/mem/metaslab.h index 2d7cd9a..6d8ff23 100644 --- a/src/mem/metaslab.h +++ b/src/mem/metaslab.h @@ -62,7 +62,7 @@ namespace snmalloc return free_queue.s.needed; } - uint8_t& sizeclass() + uint8_t sizeclass() { return free_queue.s.sizeclass; } @@ -72,6 +72,18 @@ namespace snmalloc return free_queue.s.next; } + void initialise(sizeclass_t sizeclass, Slab* slab) + { + free_queue.s.sizeclass = static_cast(sizeclass); + free_queue.init(); + // Set up meta data as if the entire slab has been turned into a free + // list. This means we don't have to check for special cases where we have + // returned all the elements, but this is a slab that is still being bump + // allocated from. Hence, the bump allocator slab will never be returned + // for use in another size class. + set_full(slab); + } + /** * Updates statistics for adding an entry to the free list, if the * slab is either @@ -91,17 +103,34 @@ namespace snmalloc bool is_full() { - auto result = get_prev() == nullptr; - SNMALLOC_ASSERT(!result || free_queue.empty()); - return result; + return get_prev() == nullptr; } - SNMALLOC_FAST_PATH void set_full() + /** + * Only wake slab if we have this many free allocations + * + * This helps remove bouncing around empty to non-empty cases. + * + * It also increases entropy, when we have randomisation. + */ + uint16_t threshold_for_waking_slab(bool is_short_slab) + { + auto capacity = get_slab_capacity(sizeclass(), is_short_slab); + uint16_t threshold = (capacity / 8) | 1; + uint16_t max = 32; + return bits::min(threshold, max); + } + + SNMALLOC_FAST_PATH void set_full(Slab* slab) { SNMALLOC_ASSERT(free_queue.empty()); - // Set needed to 1, so that "return_object" will return true after calling - // set_full - needed() = 1; + + // Prepare for the next free queue to be built. + free_queue.open(slab); + + // Set needed to at least one, possibly more so we only use + // a slab when it has a reasonable amount of free elements + needed() = threshold_for_waking_slab(Metaslab::is_short(slab)); null_prev(); } @@ -141,10 +170,8 @@ namespace snmalloc void* n = fast_free_list.take(); // Treat stealing the free list as allocating it all. - self->needed() = get_slab_capacity( - self->sizeclass(), Metaslab::is_short(Metaslab::get_slab(n))); self->remove(); - self->set_full(); + self->set_full(Metaslab::get_slab(n)); void* p = remove_cache_friendly_offset(n, self->sizeclass()); SNMALLOC_ASSERT(is_start_of_object(self, p)); @@ -173,7 +200,8 @@ namespace snmalloc if (is_full()) { - // There is no free list to validate + size_t count = free_queue.debug_length(); + SNMALLOC_ASSERT(count < threshold_for_waking_slab(is_short)); return; } @@ -187,19 +215,11 @@ namespace snmalloc // Block is not full SNMALLOC_ASSERT(SLAB_SIZE > accounted_for); - // Walk bump-free-list-segment accounting for unused space - FreeListIter fl = free_queue.terminate(); + // Account for list size + size_t count = free_queue.debug_length(); + accounted_for += count * size; - while (!fl.empty()) - { - // Check we are looking at a correctly aligned block - void* start = remove_cache_friendly_offset(fl.take(), sizeclass()); - SNMALLOC_ASSERT(((pointer_diff(slab, start) - offset) % size) == 0); - - // Account for free elements in free list - accounted_for += size; - SNMALLOC_ASSERT(SLAB_SIZE >= accounted_for); - } + SNMALLOC_ASSERT(count <= get_slab_capacity(sizeclass(), is_short)); auto bumpptr = (get_slab_capacity(sizeclass(), is_short) * size) + offset; // Check we haven't allocated more than fits in a slab diff --git a/src/mem/slab.h b/src/mem/slab.h index f617a3d..b37eedc 100644 --- a/src/mem/slab.h +++ b/src/mem/slab.h @@ -105,10 +105,12 @@ namespace snmalloc return super->dealloc_slab(self); } - SNMALLOC_ASSERT(meta.free_queue.empty()); - meta.free_queue.open(p); + meta.free_queue.add(p); - meta.needed() = allocated - 1; + // Remove trigger threshold from how many we need before we have fully + // freed the slab. + meta.needed() = + allocated - meta.threshold_for_waking_slab(Metaslab::is_short(self)); // Push on the list of slabs for this sizeclass. sl->insert_prev(&meta); diff --git a/src/mem/superslab.h b/src/mem/superslab.h index aebc856..27fa6aa 100644 --- a/src/mem/superslab.h +++ b/src/mem/superslab.h @@ -192,19 +192,13 @@ namespace snmalloc if ((self->used & 1) == 1) return alloc_slab(self, sizeclass); + Slab* slab = reinterpret_cast(self); auto& metaz = self->meta[0]; - metaz.free_queue.init(); - // Set up meta data as if the entire slab has been turned into a free - // list. This means we don't have to check for special cases where we have - // returned all the elements, but this is a slab that is still being bump - // allocated from. Hence, the bump allocator slab will never be returned - // for use in another size class. - metaz.set_full(); - metaz.sizeclass() = static_cast(sizeclass); + metaz.initialise(sizeclass, slab); self->used++; - return reinterpret_cast(self); + return slab; } // This is pre-factored to take an explicit self parameter so that we can @@ -218,14 +212,7 @@ namespace snmalloc auto& metah = self->meta[h]; uint8_t n = metah.next(); - metah.free_queue.init(); - // Set up meta data as if the entire slab has been turned into a free - // list. This means we don't have to check for special cases where we have - // returned all the elements, but this is a slab that is still being bump - // allocated from. Hence, the bump allocator slab will never be returned - // for use in another size class. - metah.set_full(); - metah.sizeclass() = static_cast(sizeclass); + metah.initialise(sizeclass, slab); self->head = h + n + 1; self->used += 2; @@ -240,7 +227,6 @@ namespace snmalloc uint8_t index = static_cast(slab_to_index(slab)); uint8_t n = head - index - 1; - meta[index].sizeclass() = 0; meta[index].next() = n; head = index; bool was_almost_full = is_almost_full();