From 66818bf632fb3197019951f9028d38c3e9da44f6 Mon Sep 17 00:00:00 2001 From: daan Date: Thu, 23 Jan 2020 09:57:55 -0800 Subject: [PATCH 01/28] use atomic yield on delayed-freeing; clarify code --- src/heap.c | 46 +++++++++++++++++++++++----------------------- src/page.c | 7 ++++--- src/segment.c | 20 ++++++++++---------- test/test-stress.c | 4 ++-- 4 files changed, 39 insertions(+), 38 deletions(-) diff --git a/src/heap.c b/src/heap.c index bdd833c3..2a4f98af 100644 --- a/src/heap.c +++ b/src/heap.c @@ -76,9 +76,9 @@ static bool mi_heap_is_valid(mi_heap_t* heap) { ----------------------------------------------------------- */ typedef enum mi_collect_e { - NORMAL, - FORCE, - ABANDON + MI_NORMAL, + MI_FORCE, + MI_ABANDON } mi_collect_t; @@ -87,12 +87,13 @@ static bool mi_heap_page_collect(mi_heap_t* heap, mi_page_queue_t* pq, mi_page_t UNUSED(heap); mi_assert_internal(mi_heap_page_is_valid(heap, pq, page, NULL, NULL)); mi_collect_t collect = *((mi_collect_t*)arg_collect); - _mi_page_free_collect(page, collect >= ABANDON); + _mi_page_free_collect(page, collect >= MI_FORCE); if (mi_page_all_free(page)) { - // no more used blocks, free the page. TODO: should we retire here and be less aggressive? - _mi_page_free(page, pq, collect != NORMAL); + // no more used blocks, free the page. + // note: this will free retired pages as well. + _mi_page_free(page, pq, collect >= MI_FORCE); } - else if (collect == ABANDON) { + else if (collect == MI_ABANDON) { // still used blocks but the thread is done; abandon the page _mi_page_abandon(page, pq); } @@ -111,61 +112,60 @@ static bool mi_heap_page_never_delayed_free(mi_heap_t* heap, mi_page_queue_t* pq static void mi_heap_collect_ex(mi_heap_t* heap, mi_collect_t collect) { if (!mi_heap_is_initialized(heap)) return; - _mi_deferred_free(heap, collect > NORMAL); + _mi_deferred_free(heap, collect >= MI_FORCE); // collect (some) abandoned pages - if (collect >= NORMAL && !heap->no_reclaim) { - if (collect == NORMAL) { + if (collect >= MI_NORMAL && !heap->no_reclaim) { + if (collect == MI_NORMAL) { // this may free some segments (but also take ownership of abandoned pages) _mi_segment_try_reclaim_abandoned(heap, false, &heap->tld->segments); } else if ( #ifdef NDEBUG - collect == FORCE + collect == MI_FORCE #else - collect >= FORCE + collect >= MI_FORCE #endif && _mi_is_main_thread() && mi_heap_is_backing(heap)) { - // the main thread is abandoned, try to free all abandoned segments. + // the main thread is abandoned (end-of-program), try to reclaim all abandoned segments. // if all memory is freed by now, all segments should be freed. _mi_segment_try_reclaim_abandoned(heap, true, &heap->tld->segments); } } // if abandoning, mark all pages to no longer add to delayed_free - if (collect == ABANDON) { - //for (mi_page_t* page = heap->pages[MI_BIN_FULL].first; page != NULL; page = page->next) { - // _mi_page_use_delayed_free(page, false); // set thread_free.delayed to MI_NO_DELAYED_FREE - //} + if (collect == MI_ABANDON) { mi_heap_visit_pages(heap, &mi_heap_page_never_delayed_free, NULL, NULL); } // free thread delayed blocks. - // (if abandoning, after this there are no more local references into the pages.) + // (if abandoning, after this there are no more thread-delayed references into the pages.) _mi_heap_delayed_free(heap); // collect all pages owned by this thread mi_heap_visit_pages(heap, &mi_heap_page_collect, &collect, NULL); - mi_assert_internal( collect != ABANDON || mi_atomic_read_ptr(mi_block_t,&heap->thread_delayed_free) == NULL ); + mi_assert_internal( collect != MI_ABANDON || mi_atomic_read_ptr(mi_block_t,&heap->thread_delayed_free) == NULL ); // collect segment caches - if (collect >= FORCE) { + if (collect >= MI_FORCE) { _mi_segment_thread_collect(&heap->tld->segments); } + #ifndef NDEBUG // collect regions - if (collect >= FORCE && _mi_is_main_thread()) { + if (collect >= MI_FORCE && _mi_is_main_thread() && mi_heap_is_backing(heap)) { _mi_mem_collect(&heap->tld->os); } + #endif } void _mi_heap_collect_abandon(mi_heap_t* heap) { - mi_heap_collect_ex(heap, ABANDON); + mi_heap_collect_ex(heap, MI_ABANDON); } void mi_heap_collect(mi_heap_t* heap, bool force) mi_attr_noexcept { - mi_heap_collect_ex(heap, (force ? FORCE : NORMAL)); + mi_heap_collect_ex(heap, (force ? MI_FORCE : MI_NORMAL)); } void mi_collect(bool force) mi_attr_noexcept { diff --git a/src/page.c b/src/page.c index fb75b826..149926e8 100644 --- a/src/page.c +++ b/src/page.c @@ -126,12 +126,12 @@ void _mi_page_use_delayed_free(mi_page_t* page, mi_delayed_t delay, bool overrid mi_thread_free_t tfreex; mi_delayed_t old_delay; do { - tfree = mi_atomic_read(&page->xthread_free); + tfree = mi_atomic_read(&page->xthread_free); // note: must acquire as we can break this loop and not do a CAS tfreex = mi_tf_set_delayed(tfree, delay); old_delay = mi_tf_delayed(tfree); if (mi_unlikely(old_delay == MI_DELAYED_FREEING)) { mi_atomic_yield(); // delay until outstanding MI_DELAYED_FREEING are done. - tfree = mi_tf_set_delayed(tfree, MI_NO_DELAYED_FREE); // will cause CAS to busy fail + // tfree = mi_tf_set_delayed(tfree, MI_NO_DELAYED_FREE); // will cause CAS to busy fail } else if (delay == old_delay) { break; // avoid atomic operation if already equal @@ -139,7 +139,8 @@ void _mi_page_use_delayed_free(mi_page_t* page, mi_delayed_t delay, bool overrid else if (!override_never && old_delay == MI_NEVER_DELAYED_FREE) { break; // leave never-delayed flag set } - } while (!mi_atomic_cas_weak(&page->xthread_free, tfreex, tfree)); + } while ((old_delay == MI_DELAYED_FREEING) || + !mi_atomic_cas_weak(&page->xthread_free, tfreex, tfree)); } /* ----------------------------------------------------------- diff --git a/src/segment.c b/src/segment.c index a76871d0..85e8817b 100644 --- a/src/segment.c +++ b/src/segment.c @@ -824,18 +824,18 @@ static void mi_segments_prepend_abandoned(mi_segment_t* first) { // first try if the abandoned list happens to be NULL if (mi_atomic_cas_ptr_weak(mi_segment_t, &abandoned, first, NULL)) return; - // if not, find the end of the list + // if not, find the end of the argument list mi_segment_t* last = first; while (last->abandoned_next != NULL) { last = last->abandoned_next; } // and atomically prepend - mi_segment_t* next; + mi_segment_t* anext; do { - next = mi_atomic_read_ptr_relaxed(mi_segment_t,&abandoned); - last->abandoned_next = next; - } while (!mi_atomic_cas_ptr_weak(mi_segment_t, &abandoned, first, next)); + anext = mi_atomic_read_ptr_relaxed(mi_segment_t,&abandoned); + last->abandoned_next = anext; + } while (!mi_atomic_cas_ptr_weak(mi_segment_t, &abandoned, first, anext)); } static void mi_segment_abandon(mi_segment_t* segment, mi_segments_tld_t* tld) { @@ -897,14 +897,14 @@ bool _mi_segment_try_reclaim_abandoned( mi_heap_t* heap, bool try_all, mi_segmen atmost--; } // split the list and push back the remaining segments - mi_segment_t* next = last->abandoned_next; + mi_segment_t* anext = last->abandoned_next; last->abandoned_next = NULL; - mi_segments_prepend_abandoned(next); + mi_segments_prepend_abandoned(anext); } // reclaim all segments that we kept while(segment != NULL) { - mi_segment_t* const next = segment->abandoned_next; // save the next segment + mi_segment_t* const anext = segment->abandoned_next; // save the next segment // got it. mi_atomic_decrement(&abandoned_count); @@ -943,7 +943,7 @@ bool _mi_segment_try_reclaim_abandoned( mi_heap_t* heap, bool try_all, mi_segmen } } mi_assert(segment->abandoned == 0); - if (segment->used == 0) { // due to page_clear + if (segment->used == 0) { // due to page_clear's mi_segment_free(segment,false,tld); } else { @@ -954,7 +954,7 @@ bool _mi_segment_try_reclaim_abandoned( mi_heap_t* heap, bool try_all, mi_segmen } // go on - segment = next; + segment = anext; } return true; diff --git a/test/test-stress.c b/test/test-stress.c index 83f9b87b..28bd4a56 100644 --- a/test/test-stress.c +++ b/test/test-stress.c @@ -277,12 +277,12 @@ static void run_os_threads(size_t nthreads) { #ifdef __cplusplus #include static void* atomic_exchange_ptr(volatile void** p, void* newval) { - return std::atomic_exchange_explicit((volatile std::atomic*)p, newval, std::memory_order_acquire); + return std::atomic_exchange((volatile std::atomic*)p, newval); } #else #include static void* atomic_exchange_ptr(volatile void** p, void* newval) { - return atomic_exchange_explicit((volatile _Atomic(void*)*)p, newval, memory_order_acquire); + return atomic_exchange((volatile _Atomic(void*)*)p, newval); } #endif From 0316859e0666bc7138e45789d71d2829656f85f3 Mon Sep 17 00:00:00 2001 From: daan Date: Thu, 23 Jan 2020 10:03:03 -0800 Subject: [PATCH 02/28] improve codegen for mi_free --- src/alloc.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/alloc.c b/src/alloc.c index 847c1830..3f577f2f 100644 --- a/src/alloc.c +++ b/src/alloc.c @@ -21,7 +21,7 @@ terms of the MIT license. A copy of the license can be found in the file // Fast allocation in a page: just pop from the free list. // Fall back to generic allocation only if the list is empty. -extern inline void* _mi_page_malloc(mi_heap_t* heap, mi_page_t* page, size_t size) mi_attr_noexcept { +extern inline void* _mi_page_malloc(mi_heap_t* heap, mi_page_t* page, size_t size) mi_attr_noexcept { mi_assert_internal(page->xblock_size==0||mi_page_block_size(page) >= size); mi_block_t* block = page->free; if (mi_unlikely(block == NULL)) { @@ -290,7 +290,8 @@ mi_block_t* _mi_page_ptr_unalign(const mi_segment_t* segment, const mi_page_t* p } -static void mi_decl_noinline mi_free_generic(const mi_segment_t* segment, mi_page_t* page, bool local, void* p) { +static void mi_decl_noinline mi_free_generic(const mi_segment_t* segment, bool local, void* p) { + mi_page_t* page = _mi_segment_page_of(segment, p); mi_block_t* block = (mi_page_has_aligned(page) ? _mi_page_ptr_unalign(segment, page, p) : (mi_block_t*)p); _mi_free_block(page, local, block); } @@ -338,7 +339,7 @@ void mi_free(void* p) mi_attr_noexcept if (mi_likely(tid == segment->thread_id && page->flags.full_aligned == 0)) { // the thread id matches and it is not a full page, nor has aligned blocks // local, and not full or aligned - mi_block_t* block = (mi_block_t*)p; + mi_block_t* const block = (mi_block_t*)p; if (mi_unlikely(mi_check_is_double_free(page,block))) return; mi_block_set_next(page, block, page->local_free); page->local_free = block; @@ -349,7 +350,8 @@ void mi_free(void* p) mi_attr_noexcept } else { // non-local, aligned blocks, or a full page; use the more generic path - mi_free_generic(segment, page, tid == segment->thread_id, p); + // note: recalc page in generic to improve code generation + mi_free_generic(segment, tid == segment->thread_id, p); } } From 6fb434a99b72838f53f75899076e3cd949b9fb57 Mon Sep 17 00:00:00 2001 From: daan Date: Thu, 23 Jan 2020 10:31:47 -0800 Subject: [PATCH 03/28] use -fvisibility=hidden on clang as well --- CMakeLists.txt | 3 +-- include/mimalloc-internal.h | 2 +- include/mimalloc.h | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 366ffc44..95318a0e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -107,10 +107,9 @@ endif() # Compiler flags if(CMAKE_C_COMPILER_ID MATCHES "AppleClang|Clang|GNU") - list(APPEND mi_cflags -Wall -Wextra -Wno-unknown-pragmas) + list(APPEND mi_cflags -Wall -Wextra -Wno-unknown-pragmas -fvisibility=hidden) if(CMAKE_C_COMPILER_ID MATCHES "GNU") list(APPEND mi_cflags -Wno-invalid-memory-model) - list(APPEND mi_cflags -fvisibility=hidden) endif() endif() diff --git a/include/mimalloc-internal.h b/include/mimalloc-internal.h index eaa327be..88a0f86d 100644 --- a/include/mimalloc-internal.h +++ b/include/mimalloc-internal.h @@ -23,7 +23,7 @@ terms of the MIT license. A copy of the license can be found in the file #if defined(_MSC_VER) #pragma warning(disable:4127) // constant conditional due to MI_SECURE paths #define mi_decl_noinline __declspec(noinline) -#elif defined(__GNUC__) || defined(__clang__) +#elif (defined(__GNUC__) && (__GNUC__>=3)) // includes clang and icc #define mi_decl_noinline __attribute__((noinline)) #else #define mi_decl_noinline diff --git a/include/mimalloc.h b/include/mimalloc.h index 1c77d462..7cf455e6 100644 --- a/include/mimalloc.h +++ b/include/mimalloc.h @@ -43,7 +43,7 @@ terms of the MIT license. A copy of the license can be found in the file #define mi_attr_alloc_size(s) #define mi_attr_alloc_size2(s1,s2) #define mi_attr_alloc_align(p) -#elif defined(__GNUC__) || defined(__clang__) +#elif defined(__GNUC__) // includes clang and icc #define mi_cdecl // leads to warnings... __attribute__((cdecl)) #define mi_decl_thread __thread #define mi_decl_export __attribute__((visibility("default"))) From cdc34595cfd3c26aa0d366fb70199509846b40db Mon Sep 17 00:00:00 2001 From: daan Date: Thu, 23 Jan 2020 10:52:27 -0800 Subject: [PATCH 04/28] fix warning in msvc --- ide/vs2017/mimalloc-override-test.vcxproj | 2 +- ide/vs2017/mimalloc.vcxproj | 4 ++-- ide/vs2019/mimalloc-override-test.vcxproj | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/ide/vs2017/mimalloc-override-test.vcxproj b/ide/vs2017/mimalloc-override-test.vcxproj index b8e2648b..faaa00e3 100644 --- a/ide/vs2017/mimalloc-override-test.vcxproj +++ b/ide/vs2017/mimalloc-override-test.vcxproj @@ -112,7 +112,7 @@ true ..\..\include MultiThreadedDebugDLL - false + Sync Default false diff --git a/ide/vs2017/mimalloc.vcxproj b/ide/vs2017/mimalloc.vcxproj index 55f37392..e08deec4 100644 --- a/ide/vs2017/mimalloc.vcxproj +++ b/ide/vs2017/mimalloc.vcxproj @@ -111,7 +111,7 @@ - Level3 + Level4 Disabled true true @@ -165,7 +165,7 @@ - Level3 + Level4 MaxSpeed true true diff --git a/ide/vs2019/mimalloc-override-test.vcxproj b/ide/vs2019/mimalloc-override-test.vcxproj index 79adedb0..a2497a19 100644 --- a/ide/vs2019/mimalloc-override-test.vcxproj +++ b/ide/vs2019/mimalloc-override-test.vcxproj @@ -90,7 +90,7 @@ true ..\..\include MultiThreadedDebugDLL - false + Sync Default false @@ -112,7 +112,7 @@ true ..\..\include MultiThreadedDebugDLL - false + Sync Default false From c9106e74a8bd50d8da2360c19741c74ac1cd0592 Mon Sep 17 00:00:00 2001 From: daan Date: Thu, 23 Jan 2020 11:06:25 -0800 Subject: [PATCH 05/28] remove __thread attribute from mimalloc.h --- include/mimalloc-internal.h | 9 ++++++--- include/mimalloc.h | 3 --- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/include/mimalloc-internal.h b/include/mimalloc-internal.h index 88a0f86d..6fca06b8 100644 --- a/include/mimalloc-internal.h +++ b/include/mimalloc-internal.h @@ -21,12 +21,15 @@ terms of the MIT license. A copy of the license can be found in the file #endif #if defined(_MSC_VER) -#pragma warning(disable:4127) // constant conditional due to MI_SECURE paths -#define mi_decl_noinline __declspec(noinline) +#pragma warning(disable:4127) // suppress constant conditional warning (due to MI_SECURE paths) +#define mi_decl_noinline __declspec(noinline) +#define mi_decl_thread __declspec(thread) #elif (defined(__GNUC__) && (__GNUC__>=3)) // includes clang and icc -#define mi_decl_noinline __attribute__((noinline)) +#define mi_decl_noinline __attribute__((noinline)) +#define mi_decl_thread __thread #else #define mi_decl_noinline +#define mi_decl_thread __thread // hope for the best :-) #endif diff --git a/include/mimalloc.h b/include/mimalloc.h index 7cf455e6..94fcd788 100644 --- a/include/mimalloc.h +++ b/include/mimalloc.h @@ -38,14 +38,12 @@ terms of the MIT license. A copy of the license can be found in the file #define mi_decl_allocator __declspec(restrict) #endif #define mi_cdecl __cdecl - #define mi_decl_thread __declspec(thread) #define mi_attr_malloc #define mi_attr_alloc_size(s) #define mi_attr_alloc_size2(s1,s2) #define mi_attr_alloc_align(p) #elif defined(__GNUC__) // includes clang and icc #define mi_cdecl // leads to warnings... __attribute__((cdecl)) - #define mi_decl_thread __thread #define mi_decl_export __attribute__((visibility("default"))) #define mi_decl_allocator #define mi_attr_malloc __attribute__((malloc)) @@ -64,7 +62,6 @@ terms of the MIT license. A copy of the license can be found in the file #endif #else #define mi_cdecl - #define mi_decl_thread __thread #define mi_decl_export #define mi_decl_allocator #define mi_attr_malloc From 76e727f7d1828d02c51b1c0266dca9eeb61ede2d Mon Sep 17 00:00:00 2001 From: daan Date: Thu, 23 Jan 2020 15:39:38 -0800 Subject: [PATCH 06/28] fix assertion on page destroy --- src/heap.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/heap.c b/src/heap.c index 2a4f98af..ab55efae 100644 --- a/src/heap.c +++ b/src/heap.c @@ -274,6 +274,9 @@ static bool _mi_heap_page_destroy(mi_heap_t* heap, mi_page_queue_t* pq, mi_page_ page->used = 0; // and free the page + // mi_page_free(page,false); + page->next = NULL; + page->prev = NULL; _mi_segment_page_free(page,false /* no force? */, &heap->tld->segments); return true; // keep going From 12701b1aac8d66ffe92bd1f80bc401d285fa32a4 Mon Sep 17 00:00:00 2001 From: daan Date: Thu, 23 Jan 2020 15:48:51 -0800 Subject: [PATCH 07/28] do not reclaim segments on collect --- src/heap.c | 31 +++++++++++++------------------ 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/src/heap.c b/src/heap.c index ab55efae..1f436b06 100644 --- a/src/heap.c +++ b/src/heap.c @@ -114,25 +114,20 @@ static void mi_heap_collect_ex(mi_heap_t* heap, mi_collect_t collect) if (!mi_heap_is_initialized(heap)) return; _mi_deferred_free(heap, collect >= MI_FORCE); - // collect (some) abandoned pages - if (collect >= MI_NORMAL && !heap->no_reclaim) { - if (collect == MI_NORMAL) { - // this may free some segments (but also take ownership of abandoned pages) - _mi_segment_try_reclaim_abandoned(heap, false, &heap->tld->segments); - } - else if ( - #ifdef NDEBUG - collect == MI_FORCE - #else - collect >= MI_FORCE - #endif - && _mi_is_main_thread() && mi_heap_is_backing(heap)) - { - // the main thread is abandoned (end-of-program), try to reclaim all abandoned segments. - // if all memory is freed by now, all segments should be freed. - _mi_segment_try_reclaim_abandoned(heap, true, &heap->tld->segments); - } + // note: never reclaim on collect but leave it to threads that need storage to reclaim + if ( + #ifdef NDEBUG + collect == MI_FORCE + #else + collect >= MI_FORCE + #endif + && _mi_is_main_thread() && mi_heap_is_backing(heap) && !heap->no_reclaim) + { + // the main thread is abandoned (end-of-program), try to reclaim all abandoned segments. + // if all memory is freed by now, all segments should be freed. + _mi_segment_try_reclaim_abandoned(heap, true, &heap->tld->segments); } + // if abandoning, mark all pages to no longer add to delayed_free if (collect == MI_ABANDON) { From f8ab4bd7dc6467ae15e1f61968a46d220d94c0d5 Mon Sep 17 00:00:00 2001 From: daan Date: Thu, 23 Jan 2020 19:49:32 -0800 Subject: [PATCH 08/28] add leak test --- test/test-stress.c | 91 ++++++++++++++++++++++++++++++++-------------- 1 file changed, 64 insertions(+), 27 deletions(-) diff --git a/test/test-stress.c b/test/test-stress.c index 28bd4a56..67ec9f05 100644 --- a/test/test-stress.c +++ b/test/test-stress.c @@ -37,11 +37,11 @@ static size_t use_one_size = 0; // use single object size of `N * s #ifdef USE_STD_MALLOC -#define custom_malloc(s) malloc(s) +#define custom_calloc(n,s) calloc(n,s) #define custom_realloc(p,s) realloc(p,s) #define custom_free(p) free(p) #else -#define custom_malloc(s) mi_malloc(s) +#define custom_calloc(n,s) mi_calloc(n,s) #define custom_realloc(p,s) mi_realloc(p,s) #define custom_free(p) mi_free(p) #endif @@ -94,9 +94,12 @@ static void* alloc_items(size_t items, random_t r) { } if (items == 40) items++; // pthreads uses that size for stack increases if (use_one_size > 0) items = (use_one_size / sizeof(uintptr_t)); - uintptr_t* p = (uintptr_t*)custom_malloc(items * sizeof(uintptr_t)); + if (items==0) items = 1; + uintptr_t* p = (uintptr_t*)custom_calloc(items,sizeof(uintptr_t)); if (p != NULL) { - for (uintptr_t i = 0; i < items; i++) p[i] = (items - i) ^ cookie; + for (uintptr_t i = 0; i < items; i++) { + p[i] = (items - i) ^ cookie; + } } return p; } @@ -126,7 +129,7 @@ static void stress(intptr_t tid) { void** data = NULL; size_t data_size = 0; size_t data_top = 0; - void** retained = (void**)custom_malloc(retain * sizeof(void*)); + void** retained = (void**)custom_calloc(retain,sizeof(void*)); size_t retain_top = 0; while (allocs > 0 || retain > 0) { @@ -171,7 +174,45 @@ static void stress(intptr_t tid) { //bench_end_thread(); } -static void run_os_threads(size_t nthreads); +static void run_os_threads(size_t nthreads, void (*entry)(intptr_t tid)); + +static void test_stress(void) { + uintptr_t r = 43 * 43; + for (int n = 0; n < ITER; n++) { + run_os_threads(THREADS, &stress); + for (int i = 0; i < TRANSFERS; i++) { + if (chance(50, &r) || n + 1 == ITER) { // free all on last run, otherwise free half of the transfers + void* p = atomic_exchange_ptr(&transfer[i], NULL); + free_items(p); + } + } + mi_collect(false); +#ifndef NDEBUG + if ((n + 1) % 10 == 0) { printf("- iterations left: %3d\n", ITER - (n + 1)); } +#endif + } +} + +static void leak(intptr_t tid) { + uintptr_t r = 43*tid; + void* p = alloc_items(pick(&r)%128, &r); + if (chance(10, &r)) { + intptr_t i = (pick(&r) % TRANSFERS); + void* q = atomic_exchange_ptr(&transfer[i], p); + free_items(q); + } +} + +static void test_leak(void) { + for (int n = 0; n < ITER; n++) { + run_os_threads(THREADS, &leak); + mi_collect(false); +#ifndef NDEBUG + //if ((n + 1) % 10 == 0) + { printf("- iterations left: %3d\n", ITER - (n + 1)); } +#endif + } +} int main(int argc, char** argv) { // > mimalloc-test-stress [THREADS] [SCALE] [ITER] @@ -198,19 +239,11 @@ int main(int argc, char** argv) { // Run ITER full iterations where half the objects in the transfer buffer survive to the next round. mi_stats_reset(); - uintptr_t r = 43 * 43; - for (int n = 0; n < ITER; n++) { - run_os_threads(THREADS); - for (int i = 0; i < TRANSFERS; i++) { - if (chance(50, &r) || n + 1 == ITER) { // free all on last run, otherwise free half of the transfers - void* p = atomic_exchange_ptr(&transfer[i], NULL); - free_items(p); - } - } - mi_collect(false); -#ifndef NDEBUG - if ((n + 1) % 10 == 0) { printf("- iterations left: %3d\n", ITER - (n + 1)); } -#endif + if (true) { + test_stress(); + } + else { + test_leak(); } mi_collect(true); @@ -220,18 +253,21 @@ int main(int argc, char** argv) { } +static void (*thread_entry_fun)(intptr_t) = &stress; + #ifdef _WIN32 #include static DWORD WINAPI thread_entry(LPVOID param) { - stress((intptr_t)param); + thread_entry_fun((intptr_t)param); return 0; } -static void run_os_threads(size_t nthreads) { - DWORD* tids = (DWORD*)custom_malloc(nthreads * sizeof(DWORD)); - HANDLE* thandles = (HANDLE*)custom_malloc(nthreads * sizeof(HANDLE)); +static void run_os_threads(size_t nthreads, void (*fun)(intptr_t)) { + thread_entry_fun = fun; + DWORD* tids = (DWORD*)custom_calloc(nthreads,sizeof(DWORD)); + HANDLE* thandles = (HANDLE*)custom_calloc(nthreads,sizeof(HANDLE)); for (uintptr_t i = 0; i < nthreads; i++) { thandles[i] = CreateThread(0, 4096, &thread_entry, (void*)(i), 0, &tids[i]); } @@ -246,7 +282,7 @@ static void run_os_threads(size_t nthreads) { } static void* atomic_exchange_ptr(volatile void** p, void* newval) { -#if (INTPTR_MAX == UINT32_MAX) +#if (INTPTR_MAX == INT32_MAX) return (void*)InterlockedExchange((volatile LONG*)p, (LONG)newval); #else return (void*)InterlockedExchange64((volatile LONG64*)p, (LONG64)newval); @@ -257,12 +293,13 @@ static void* atomic_exchange_ptr(volatile void** p, void* newval) { #include static void* thread_entry(void* param) { - stress((uintptr_t)param); + thread_entry_fun((uintptr_t)param); return NULL; } -static void run_os_threads(size_t nthreads) { - pthread_t* threads = (pthread_t*)custom_malloc(nthreads * sizeof(pthread_t)); +static void run_os_threads(size_t nthreads, void (*fun)(intptr_t)) { + thread_entry_fun = fun; + pthread_t* threads = (pthread_t*)custom_calloc(nthreads,sizeof(pthread_t)); memset(threads, 0, sizeof(pthread_t) * nthreads); //pthread_setconcurrency(nthreads); for (uintptr_t i = 0; i < nthreads; i++) { From 4a2a0c2d503ad5334555f4f86d7f0128b3676aae Mon Sep 17 00:00:00 2001 From: daan Date: Thu, 23 Jan 2020 19:50:35 -0800 Subject: [PATCH 09/28] initial abandon based on fine-grained reclamation --- include/mimalloc-internal.h | 7 +- src/heap.c | 2 +- src/memory.c | 4 +- src/page.c | 49 ++--- src/segment.c | 365 +++++++++++++++++++++++++----------- 5 files changed, 289 insertions(+), 138 deletions(-) diff --git a/include/mimalloc-internal.h b/include/mimalloc-internal.h index 6fca06b8..3335414a 100644 --- a/include/mimalloc-internal.h +++ b/include/mimalloc-internal.h @@ -75,12 +75,13 @@ bool _mi_mem_unprotect(void* addr, size_t size); void _mi_mem_collect(mi_os_tld_t* tld); // "segment.c" -mi_page_t* _mi_segment_page_alloc(size_t block_wsize, mi_segments_tld_t* tld, mi_os_tld_t* os_tld); +mi_page_t* _mi_segment_page_alloc(mi_heap_t* heap, size_t block_wsize, mi_segments_tld_t* tld, mi_os_tld_t* os_tld); void _mi_segment_page_free(mi_page_t* page, bool force, mi_segments_tld_t* tld); void _mi_segment_page_abandon(mi_page_t* page, mi_segments_tld_t* tld); -bool _mi_segment_try_reclaim_abandoned( mi_heap_t* heap, bool try_all, mi_segments_tld_t* tld); -void _mi_segment_thread_collect(mi_segments_tld_t* tld); uint8_t* _mi_segment_page_start(const mi_segment_t* segment, const mi_page_t* page, size_t block_size, size_t* page_size, size_t* pre_size); // page start for any page +void _mi_segment_thread_collect(mi_segments_tld_t* tld); +void _mi_abandoned_reclaim_all(mi_heap_t* heap, mi_segments_tld_t* tld); +void _mi_abandoned_await_readers(void); // "page.c" void* _mi_malloc_generic(mi_heap_t* heap, size_t size) mi_attr_noexcept mi_attr_malloc; diff --git a/src/heap.c b/src/heap.c index 1f436b06..e76a147c 100644 --- a/src/heap.c +++ b/src/heap.c @@ -125,7 +125,7 @@ static void mi_heap_collect_ex(mi_heap_t* heap, mi_collect_t collect) { // the main thread is abandoned (end-of-program), try to reclaim all abandoned segments. // if all memory is freed by now, all segments should be freed. - _mi_segment_try_reclaim_abandoned(heap, true, &heap->tld->segments); + _mi_abandoned_reclaim_all(heap, &heap->tld->segments); } diff --git a/src/memory.c b/src/memory.c index a442a35d..c7388054 100644 --- a/src/memory.c +++ b/src/memory.c @@ -419,6 +419,7 @@ void _mi_mem_free(void* p, size_t size, size_t id, bool full_commit, bool any_re bool any_unreset; mi_bitmap_claim(®ion->reset, 1, blocks, bit_idx, &any_unreset); if (any_unreset) { + _mi_abandoned_await_readers(); // ensure no more pending write (in case reset = decommit) _mi_mem_reset(p, blocks * MI_SEGMENT_SIZE, tld); } } @@ -451,7 +452,8 @@ void _mi_mem_collect(mi_os_tld_t* tld) { memset(®ions[i], 0, sizeof(mem_region_t)); // and release the whole region mi_atomic_write(®ion->info, 0); - if (start != NULL) { // && !_mi_os_is_huge_reserved(start)) { + if (start != NULL) { // && !_mi_os_is_huge_reserved(start)) { + _mi_abandoned_await_readers(); // ensure no pending reads _mi_arena_free(start, MI_REGION_SIZE, arena_memid, tld->stats); } } diff --git a/src/page.c b/src/page.c index 149926e8..c5b86b08 100644 --- a/src/page.c +++ b/src/page.c @@ -37,7 +37,7 @@ static inline mi_block_t* mi_page_block_at(const mi_page_t* page, void* page_sta } static void mi_page_init(mi_heap_t* heap, mi_page_t* page, size_t size, mi_tld_t* tld); - +static void mi_page_extend_free(mi_heap_t* heap, mi_page_t* page, mi_tld_t* tld); #if (MI_DEBUG>=3) static size_t mi_page_list_count(mi_page_t* page, mi_block_t* head) { @@ -242,32 +242,37 @@ void _mi_page_reclaim(mi_heap_t* heap, mi_page_t* page) { // allocate a fresh page from a segment static mi_page_t* mi_page_fresh_alloc(mi_heap_t* heap, mi_page_queue_t* pq, size_t block_size) { mi_assert_internal(pq==NULL||mi_heap_contains_queue(heap, pq)); - mi_page_t* page = _mi_segment_page_alloc(block_size, &heap->tld->segments, &heap->tld->os); - if (page == NULL) return NULL; - mi_assert_internal(pq==NULL || _mi_page_segment(page)->page_kind != MI_PAGE_HUGE); - mi_page_init(heap, page, block_size, heap->tld); - _mi_stat_increase( &heap->tld->stats.pages, 1); - if (pq!=NULL) mi_page_queue_push(heap, pq, page); // huge pages use pq==NULL - mi_assert_expensive(_mi_page_is_valid(page)); - return page; + mi_assert_internal(pq==NULL||block_size == pq->block_size); + mi_page_t* page = _mi_segment_page_alloc(heap, block_size, &heap->tld->segments, &heap->tld->os); + if (page == NULL) { + // this may be out-of-memory, or a page was reclaimed + if (pq!=NULL && (page = pq->first) != NULL) { + mi_assert_expensive(_mi_page_is_valid(page)); + if (!mi_page_immediate_available(page)) { + mi_page_extend_free(heap, page, heap->tld); + } + mi_assert_internal(mi_page_immediate_available(page)); + if (mi_page_immediate_available(page)) { + return page; // reclaimed page + } + } + return NULL; // out-of-memory + } + else { + // a fresh page was allocated, initialize it + mi_assert_internal(pq==NULL || _mi_page_segment(page)->page_kind != MI_PAGE_HUGE); + mi_page_init(heap, page, block_size, heap->tld); + _mi_stat_increase(&heap->tld->stats.pages, 1); + if (pq!=NULL) mi_page_queue_push(heap, pq, page); // huge pages use pq==NULL + mi_assert_expensive(_mi_page_is_valid(page)); + return page; + } } // Get a fresh page to use static mi_page_t* mi_page_fresh(mi_heap_t* heap, mi_page_queue_t* pq) { mi_assert_internal(mi_heap_contains_queue(heap, pq)); - - // try to reclaim an abandoned page first - mi_page_t* page = pq->first; - if (!heap->no_reclaim && - _mi_segment_try_reclaim_abandoned(heap, false, &heap->tld->segments) && - page != pq->first) - { - // we reclaimed, and we got lucky with a reclaimed page in our queue - page = pq->first; - if (page->free != NULL) return page; - } - // otherwise allocate the page - page = mi_page_fresh_alloc(heap, pq, pq->block_size); + mi_page_t* page = mi_page_fresh_alloc(heap, pq, pq->block_size); if (page==NULL) return NULL; mi_assert_internal(pq->block_size==mi_page_block_size(page)); mi_assert_internal(pq==mi_page_queue(heap, mi_page_block_size(page))); diff --git a/src/segment.c b/src/segment.c index 85e8817b..95ae6d8b 100644 --- a/src/segment.c +++ b/src/segment.c @@ -743,7 +743,9 @@ static void mi_segment_page_claim(mi_segment_t* segment, mi_page_t* page, mi_seg static void mi_segment_abandon(mi_segment_t* segment, mi_segments_tld_t* tld); -static void mi_segment_page_clear(mi_segment_t* segment, mi_page_t* page, mi_segments_tld_t* tld) { +// clear page data; can be called on abandoned segments +static void mi_segment_page_clear(mi_segment_t* segment, mi_page_t* page, bool allow_reset, mi_segments_tld_t* tld) +{ mi_assert_internal(page->segment_in_use); mi_assert_internal(mi_page_all_free(page)); mi_assert_internal(page->is_committed); @@ -773,7 +775,7 @@ static void mi_segment_page_clear(mi_segment_t* segment, mi_page_t* page, mi_seg segment->used--; // add to the free page list for reuse/reset - if (segment->page_kind <= MI_PAGE_MEDIUM) { + if (allow_reset && segment->page_kind <= MI_PAGE_MEDIUM) { mi_pages_reset_add(segment, page, tld); } } @@ -786,7 +788,7 @@ void _mi_segment_page_free(mi_page_t* page, bool force, mi_segments_tld_t* tld) mi_reset_delayed(tld); // mark it as free now - mi_segment_page_clear(segment, page, tld); + mi_segment_page_clear(segment, page, true, tld); if (segment->used == 0) { // no more used pages; remove from the free list and free the segment @@ -814,39 +816,122 @@ void _mi_segment_page_free(mi_page_t* page, bool force, mi_segments_tld_t* tld) // live blocks (reached through other threads). Such segments // are "abandoned" and will be reclaimed by other threads to // reuse their pages and/or free them eventually -static volatile _Atomic(mi_segment_t*) abandoned; // = NULL; -static volatile _Atomic(uintptr_t) abandoned_count; // = 0; approximate count of abandoned segments -// prepend a list of abandoned segments atomically to the global abandoned list; O(n) -static void mi_segments_prepend_abandoned(mi_segment_t* first) { - if (first == NULL) return; - // first try if the abandoned list happens to be NULL - if (mi_atomic_cas_ptr_weak(mi_segment_t, &abandoned, first, NULL)) return; +#define MI_TAGGED_MASK MI_SEGMENT_MASK +typedef uintptr_t mi_tagged_segment_t; - // if not, find the end of the argument list +static mi_segment_t* mi_tagged_segment_ptr(mi_tagged_segment_t ts) { + return (mi_segment_t*)(ts & ~MI_TAGGED_MASK); +} + +static mi_tagged_segment_t mi_tagged_segment(mi_segment_t* segment, mi_tagged_segment_t ts) { + mi_assert_internal(((uintptr_t)segment & MI_TAGGED_MASK) == 0); + if (segment==NULL) return 0; // no need to tag NULL + uintptr_t tag = ((ts & MI_TAGGED_MASK) + 1) & MI_TAGGED_MASK; + return ((uintptr_t)segment | tag); +} + +static volatile _Atomic(mi_segment_t*) abandoned_visited; // = NULL +static volatile _Atomic(mi_tagged_segment_t) abandoned; // = NULL +static volatile _Atomic(uintptr_t) abandoned_readers; // = 0 + +static void mi_abandoned_visited_push(mi_segment_t* segment) { + mi_assert_internal(segment->thread_id == 0); + mi_assert_internal(segment->abandoned_next == NULL); + mi_assert_internal(segment->next == NULL && segment->prev == NULL); + mi_assert_internal(segment->used > 0); + mi_segment_t* anext; + do { + anext = mi_atomic_read_ptr_relaxed(mi_segment_t, &abandoned_visited); + segment->abandoned_next = anext; + } while (!mi_atomic_cas_ptr_weak(mi_segment_t, &abandoned_visited, segment, anext)); +} + +static bool mi_abandoned_visited_revisit(void) { + // grab the whole visited list + mi_segment_t* first = mi_atomic_exchange_ptr(mi_segment_t, &abandoned_visited, NULL); + if (first == NULL) return false; + + // first try to swap directly if the abandoned list happens to be NULL + mi_tagged_segment_t afirst = mi_tagged_segment(first,0); + if (mi_atomic_cas_weak(&abandoned, afirst, 0)) return true; + + // find the last element of the visited list: O(n) mi_segment_t* last = first; while (last->abandoned_next != NULL) { last = last->abandoned_next; } - // and atomically prepend - mi_segment_t* anext; + // and atomically prepend to the abandoned list + // (no need to increase the readers as we don't access the abandoned segments) + mi_tagged_segment_t anext; do { - anext = mi_atomic_read_ptr_relaxed(mi_segment_t,&abandoned); - last->abandoned_next = anext; - } while (!mi_atomic_cas_ptr_weak(mi_segment_t, &abandoned, first, anext)); + anext = mi_atomic_read_relaxed(&abandoned); + last->abandoned_next = mi_tagged_segment_ptr(anext); + afirst = mi_tagged_segment(first, anext); + } while (!mi_atomic_cas_weak(&abandoned, afirst, anext)); + return true; } +static void mi_abandoned_push(mi_segment_t* segment) { + mi_assert_internal(segment->thread_id == 0); + mi_assert_internal(segment->abandoned_next == NULL); + mi_assert_internal(segment->next == NULL && segment->prev == NULL); + mi_assert_internal(segment->used > 0); + mi_tagged_segment_t ts; + mi_tagged_segment_t next; + do { + ts = mi_atomic_read_relaxed(&abandoned); + segment->abandoned_next = mi_tagged_segment_ptr(ts); + next = mi_tagged_segment(segment, ts); + } while (!mi_atomic_cas_weak(&abandoned, next, ts)); +} + +void _mi_abandoned_await_readers(void) { + uintptr_t n; + do { + n = mi_atomic_read(&abandoned_readers); + if (n != 0) mi_atomic_yield(); + } while (n != 0); +} + +static mi_segment_t* mi_abandoned_pop(void) { + mi_segment_t* segment; + mi_tagged_segment_t ts = mi_atomic_read_relaxed(&abandoned); + segment = mi_tagged_segment_ptr(ts); + if (segment == NULL) { + if (!mi_abandoned_visited_revisit()) return NULL; // try to swap in the visited list on NULL + } + // Do a pop. We use a reader lock to prevent + // a segment to be decommitted while a read is still pending, and a tagged + // pointer to prevent A-B-A link corruption. + mi_atomic_increment(&abandoned_readers); // ensure no segment gets decommitted + mi_tagged_segment_t next = 0; + do { + ts = mi_atomic_read_relaxed(&abandoned); + segment = mi_tagged_segment_ptr(ts); + if (segment != NULL) { + next = mi_tagged_segment(segment->abandoned_next, ts); // note: reads segment so should not be decommitted + } + } while (segment != NULL && !mi_atomic_cas_weak(&abandoned, next, ts)); + mi_atomic_decrement(&abandoned_readers); // release reader lock + if (segment != NULL) { + segment->abandoned_next = NULL; + } + return segment; +} + + static void mi_segment_abandon(mi_segment_t* segment, mi_segments_tld_t* tld) { mi_assert_internal(segment->used == segment->abandoned); mi_assert_internal(segment->used > 0); mi_assert_internal(segment->abandoned_next == NULL); - mi_assert_expensive(mi_segment_is_valid(segment,tld)); + mi_assert_expensive(mi_segment_is_valid(segment, tld)); // remove the segment from the free page queue if needed - mi_reset_delayed(tld); - mi_pages_reset_remove_all_in_segment(segment, mi_option_is_enabled(mi_option_abandoned_page_reset), tld); + mi_reset_delayed(tld); + mi_pages_reset_remove_all_in_segment(segment, mi_option_is_enabled(mi_option_abandoned_page_reset), tld); mi_segment_remove_from_free_queue(segment, tld); mi_assert_internal(segment->next == NULL && segment->prev == NULL); @@ -855,8 +940,7 @@ static void mi_segment_abandon(mi_segment_t* segment, mi_segments_tld_t* tld) { mi_segments_track_size(-((long)segment->segment_size), tld); segment->thread_id = 0; segment->abandoned_next = NULL; - mi_segments_prepend_abandoned(segment); // prepend one-element list - mi_atomic_increment(&abandoned_count); // keep approximate count + mi_abandoned_push(segment); } void _mi_segment_page_abandon(mi_page_t* page, mi_segments_tld_t* tld) { @@ -865,107 +949,164 @@ void _mi_segment_page_abandon(mi_page_t* page, mi_segments_tld_t* tld) { mi_assert_internal(mi_page_heap(page) == NULL); mi_segment_t* segment = _mi_page_segment(page); mi_assert_expensive(!mi_pages_reset_contains(page, tld)); - mi_assert_expensive(mi_segment_is_valid(segment,tld)); + mi_assert_expensive(mi_segment_is_valid(segment, tld)); segment->abandoned++; _mi_stat_increase(&tld->stats->pages_abandoned, 1); mi_assert_internal(segment->abandoned <= segment->used); if (segment->used == segment->abandoned) { // all pages are abandoned, abandon the entire segment - mi_segment_abandon(segment,tld); + mi_segment_abandon(segment, tld); } } -bool _mi_segment_try_reclaim_abandoned( mi_heap_t* heap, bool try_all, mi_segments_tld_t* tld) { - // To avoid the A-B-A problem, grab the entire list atomically - mi_segment_t* segment = mi_atomic_read_ptr_relaxed(mi_segment_t,&abandoned); // pre-read to avoid expensive atomic operations - if (segment == NULL) return false; - segment = mi_atomic_exchange_ptr(mi_segment_t, &abandoned, NULL); - if (segment == NULL) return false; - // we got a non-empty list - if (!try_all) { - // take at most 1/8th of the list and append the rest back to the abandoned list again - // this is O(n) but simplifies the code a lot (as we don't have an A-B-A problem) - // and probably ok since the length will tend to be not too large. - uintptr_t atmost = mi_atomic_read(&abandoned_count)/8; // at most 1/8th of all outstanding (estimated) - if (atmost < 8) atmost = 8; // but at least 8 - - // find the split point - mi_segment_t* last = segment; - while (last->abandoned_next != NULL && atmost > 0) { - last = last->abandoned_next; - atmost--; - } - // split the list and push back the remaining segments - mi_segment_t* anext = last->abandoned_next; - last->abandoned_next = NULL; - mi_segments_prepend_abandoned(anext); - } - - // reclaim all segments that we kept - while(segment != NULL) { - mi_segment_t* const anext = segment->abandoned_next; // save the next segment - - // got it. - mi_atomic_decrement(&abandoned_count); - segment->thread_id = _mi_thread_id(); - segment->abandoned_next = NULL; - mi_segments_track_size((long)segment->segment_size,tld); - mi_assert_internal(segment->next == NULL && segment->prev == NULL); - mi_assert_expensive(mi_segment_is_valid(segment,tld)); - _mi_stat_decrease(&tld->stats->segments_abandoned,1); - - // add its abandoned pages to the current thread - mi_assert(segment->abandoned == segment->used); - for (size_t i = 0; i < segment->capacity; i++) { - mi_page_t* page = &segment->pages[i]; - if (page->segment_in_use) { - mi_assert_internal(!page->is_reset); - mi_assert_internal(page->is_committed); - mi_assert_internal(mi_page_not_in_queue(page, tld)); - mi_assert_internal(mi_page_thread_free_flag(page)==MI_NEVER_DELAYED_FREE); - mi_assert_internal(mi_page_heap(page) == NULL); +static bool mi_segment_pages_collect(mi_segment_t* segment, size_t block_size, mi_segments_tld_t* tld) +{ + mi_assert_internal(block_size < MI_HUGE_BLOCK_SIZE); + bool has_page = false; + for (size_t i = 0; i < segment->capacity; i++) { + mi_page_t* page = &segment->pages[i]; + if (page->segment_in_use) { + mi_assert_internal(!page->is_reset); + mi_assert_internal(page->is_committed); + mi_assert_internal(mi_page_not_in_queue(page, tld)); + mi_assert_internal(mi_page_thread_free_flag(page)==MI_NEVER_DELAYED_FREE); + mi_assert_internal(mi_page_heap(page) == NULL); + mi_assert_internal(page->next == NULL); + // ensure used count is up to date and collect potential concurrent frees + _mi_page_free_collect(page, false); + if (mi_page_all_free(page)) { + // if everything free already, clear the page directly segment->abandoned--; - mi_assert(page->next == NULL); _mi_stat_decrease(&tld->stats->pages_abandoned, 1); - // set the heap again and allow delayed free again - mi_page_set_heap(page, heap); - _mi_page_use_delayed_free(page, MI_USE_DELAYED_FREE, true); // override never (after heap is set) - _mi_page_free_collect(page, false); // ensure used count is up to date - if (mi_page_all_free(page)) { - // if everything free already, clear the page directly - mi_segment_page_clear(segment,page,tld); - } - else { - // otherwise reclaim it into the heap - _mi_page_reclaim(heap,page); - } + mi_segment_page_clear(segment, page, false, tld); // no reset allowed (as the segment is still abandoned) + has_page = true; + } + else if (page->xblock_size == block_size && page->used < page->reserved) { + // a page has available free blocks of the right size + has_page = true; } } - mi_assert(segment->abandoned == 0); - if (segment->used == 0) { // due to page_clear's - mi_segment_free(segment,false,tld); + } + return has_page; +} + +#define MI_RECLAIMED ((mi_segment_t*)1) + +static mi_segment_t* mi_segment_reclaim(mi_segment_t* segment, mi_heap_t* heap, size_t block_size, mi_page_kind_t page_kind, mi_segments_tld_t* tld) { + UNUSED_RELEASE(page_kind); + mi_assert_internal(page_kind == segment->page_kind); + mi_assert_internal(segment->abandoned_next == NULL); + bool right_page_reclaimed = false; + + segment->thread_id = _mi_thread_id(); + mi_segments_track_size((long)segment->segment_size, tld); + mi_assert_internal(segment->next == NULL && segment->prev == NULL); + mi_assert_expensive(mi_segment_is_valid(segment, tld)); + _mi_stat_decrease(&tld->stats->segments_abandoned, 1); + + for (size_t i = 0; i < segment->capacity; i++) { + mi_page_t* page = &segment->pages[i]; + if (page->segment_in_use) { + mi_assert_internal(!page->is_reset); + mi_assert_internal(page->is_committed); + mi_assert_internal(mi_page_not_in_queue(page, tld)); + mi_assert_internal(mi_page_thread_free_flag(page)==MI_NEVER_DELAYED_FREE); + mi_assert_internal(mi_page_heap(page) == NULL); + segment->abandoned--; + mi_assert(page->next == NULL); + _mi_stat_decrease(&tld->stats->pages_abandoned, 1); + // set the heap again and allow delayed free again + mi_page_set_heap(page, heap); + _mi_page_use_delayed_free(page, MI_USE_DELAYED_FREE, true); // override never (after heap is set) + mi_assert_internal(!mi_page_all_free(page)); + // TODO: should we not collect again given that we just collected? + _mi_page_free_collect(page, false); // ensure used count is up to date + if (mi_page_all_free(page)) { + // if everything free already, clear the page directly + mi_segment_page_clear(segment, page, true, tld); // reset is ok now + } + else { + // otherwise reclaim it into the heap + _mi_page_reclaim(heap, page); + if (block_size == page->xblock_size) { + right_page_reclaimed = true; + } + } + } + } + mi_assert_internal(segment->abandoned == 0); + if (right_page_reclaimed) { + // add the segment's free pages to the free small segment queue + if (segment->page_kind <= MI_PAGE_MEDIUM && mi_segment_has_free(segment)) { + mi_segment_insert_in_free_queue(segment, tld); + } + // and return reclaimed: at the page allocation the page is already in the queue now + return MI_RECLAIMED; + } + else { + // otherwise return the segment as it will contain some free pages + mi_assert_internal(segment->used < segment->capacity); + return segment; + } +} + +static mi_segment_t* mi_segment_try_reclaim(mi_heap_t* heap, size_t block_size, mi_page_kind_t page_kind, mi_segments_tld_t* tld) +{ + mi_segment_t* segment; + int max_tries = 8; // limit the work to bound allocation times + while ((max_tries-- > 0) && ((segment = mi_abandoned_pop()) != NULL)) { + bool has_page = mi_segment_pages_collect(segment,block_size,tld); // try to free up pages (due to concurrent frees) + if (has_page && segment->page_kind == page_kind) { + // found a free page of the right kind, or page of the right block_size with free space + return mi_segment_reclaim(segment, heap, block_size, page_kind, tld); + } + else if (segment->used==0) { + // free the segment to make it available to other threads + mi_segment_os_free(segment, segment->segment_size, tld); } else { - // add its free pages to the the current thread free small segment queue - if (segment->page_kind <= MI_PAGE_MEDIUM && mi_segment_has_free(segment)) { - mi_segment_insert_in_free_queue(segment,tld); - } + // push on the visited list so it gets not looked at too quickly again + mi_abandoned_visited_push(segment); } - - // go on - segment = anext; } - - return true; + return NULL; } +static mi_segment_t* mi_segment_reclaim_or_alloc(mi_heap_t* heap, size_t block_size, mi_page_kind_t page_kind, size_t page_shift, mi_segments_tld_t* tld, mi_os_tld_t* os_tld) +{ + mi_assert_internal(page_kind <= MI_PAGE_LARGE); + mi_assert_internal(block_size < MI_HUGE_BLOCK_SIZE); + mi_segment_t* segment = mi_segment_try_reclaim(heap, block_size, page_kind, tld); + if (segment == MI_RECLAIMED) { + return NULL; // pretend out-of-memory as the page will be in the page queue + } + else if (segment == NULL) { + return mi_segment_alloc(0, page_kind, page_shift, tld, os_tld); + } + else { + return segment; + } +} + +void _mi_abandoned_reclaim_all(mi_heap_t* heap, mi_segments_tld_t* tld) { + mi_segment_t* segment; + while ((segment = mi_abandoned_pop()) != NULL) { + mi_segment_t* res = mi_segment_reclaim(segment, heap, 0, segment->page_kind, tld); + mi_assert_internal(res != NULL); + if (res != MI_RECLAIMED && res != NULL) { + mi_assert_internal(res == segment); + if (segment->page_kind <= MI_PAGE_MEDIUM && mi_segment_has_free(segment)) { + mi_segment_insert_in_free_queue(segment, tld); + } + } + } +} /* ----------------------------------------------------------- Small page allocation ----------------------------------------------------------- */ - static mi_page_t* mi_segment_find_free(mi_segment_t* segment, mi_segments_tld_t* tld) { mi_assert_internal(mi_segment_has_free(segment)); mi_assert_expensive(mi_segment_is_valid(segment, tld)); @@ -986,13 +1127,15 @@ static mi_page_t* mi_segment_page_alloc_in(mi_segment_t* segment, mi_segments_tl return mi_segment_find_free(segment, tld); } -static mi_page_t* mi_segment_page_alloc(mi_page_kind_t kind, size_t page_shift, mi_segments_tld_t* tld, mi_os_tld_t* os_tld) { +static mi_page_t* mi_segment_page_alloc(mi_heap_t* heap, size_t block_size, mi_page_kind_t kind, size_t page_shift, mi_segments_tld_t* tld, mi_os_tld_t* os_tld) { // find an available segment the segment free queue mi_segment_queue_t* const free_queue = mi_segment_free_queue_of_kind(kind, tld); if (mi_segment_queue_is_empty(free_queue)) { // possibly allocate a fresh segment - mi_segment_t* segment = mi_segment_alloc(0, kind, page_shift, tld, os_tld); - if (segment == NULL) return NULL; // return NULL if out-of-memory + mi_segment_t* segment = mi_segment_reclaim_or_alloc(heap, block_size, kind, page_shift, tld, os_tld); + if (segment == NULL) return NULL; // return NULL if out-of-memory (or reclaimed) + mi_assert_internal(segment->page_kind==kind); + mi_assert_internal(segment->used < segment->capacity); mi_segment_enqueue(free_queue, segment); } mi_assert_internal(free_queue->first != NULL); @@ -1005,20 +1148,20 @@ static mi_page_t* mi_segment_page_alloc(mi_page_kind_t kind, size_t page_shift, return page; } -static mi_page_t* mi_segment_small_page_alloc(mi_segments_tld_t* tld, mi_os_tld_t* os_tld) { - return mi_segment_page_alloc(MI_PAGE_SMALL,MI_SMALL_PAGE_SHIFT,tld,os_tld); +static mi_page_t* mi_segment_small_page_alloc(mi_heap_t* heap, size_t block_size, mi_segments_tld_t* tld, mi_os_tld_t* os_tld) { + return mi_segment_page_alloc(heap, block_size, MI_PAGE_SMALL,MI_SMALL_PAGE_SHIFT,tld,os_tld); } -static mi_page_t* mi_segment_medium_page_alloc(mi_segments_tld_t* tld, mi_os_tld_t* os_tld) { - return mi_segment_page_alloc(MI_PAGE_MEDIUM, MI_MEDIUM_PAGE_SHIFT, tld, os_tld); +static mi_page_t* mi_segment_medium_page_alloc(mi_heap_t* heap, size_t block_size, mi_segments_tld_t* tld, mi_os_tld_t* os_tld) { + return mi_segment_page_alloc(heap, block_size, MI_PAGE_MEDIUM, MI_MEDIUM_PAGE_SHIFT, tld, os_tld); } /* ----------------------------------------------------------- large page allocation ----------------------------------------------------------- */ -static mi_page_t* mi_segment_large_page_alloc(mi_segments_tld_t* tld, mi_os_tld_t* os_tld) { - mi_segment_t* segment = mi_segment_alloc(0,MI_PAGE_LARGE,MI_LARGE_PAGE_SHIFT,tld,os_tld); +static mi_page_t* mi_segment_large_page_alloc(mi_heap_t* heap, size_t block_size, mi_segments_tld_t* tld, mi_os_tld_t* os_tld) { + mi_segment_t* segment = mi_segment_reclaim_or_alloc(heap,block_size,MI_PAGE_LARGE,MI_LARGE_PAGE_SHIFT,tld,os_tld); if (segment == NULL) return NULL; mi_page_t* page = mi_segment_find_free(segment, tld); mi_assert_internal(page != NULL); @@ -1043,16 +1186,16 @@ static mi_page_t* mi_segment_huge_page_alloc(size_t size, mi_segments_tld_t* tld Page allocation and free ----------------------------------------------------------- */ -mi_page_t* _mi_segment_page_alloc(size_t block_size, mi_segments_tld_t* tld, mi_os_tld_t* os_tld) { +mi_page_t* _mi_segment_page_alloc(mi_heap_t* heap, size_t block_size, mi_segments_tld_t* tld, mi_os_tld_t* os_tld) { mi_page_t* page; if (block_size <= MI_SMALL_OBJ_SIZE_MAX) { - page = mi_segment_small_page_alloc(tld,os_tld); + page = mi_segment_small_page_alloc(heap, block_size, tld, os_tld); } else if (block_size <= MI_MEDIUM_OBJ_SIZE_MAX) { - page = mi_segment_medium_page_alloc(tld, os_tld); + page = mi_segment_medium_page_alloc(heap, block_size, tld, os_tld); } else if (block_size <= MI_LARGE_OBJ_SIZE_MAX) { - page = mi_segment_large_page_alloc(tld, os_tld); + page = mi_segment_large_page_alloc(heap, block_size, tld, os_tld); } else { page = mi_segment_huge_page_alloc(block_size,tld,os_tld); From 58fdcbb0cd6fbe426237f334ba4a7cf8decebf35 Mon Sep 17 00:00:00 2001 From: daan Date: Thu, 23 Jan 2020 21:37:14 -0800 Subject: [PATCH 10/28] fix bug in collect where has_page was not set on free pages --- src/options.c | 2 +- src/segment.c | 19 ++++++++++++++----- test/test-stress.c | 29 +++++++++++++++++++++++++---- 3 files changed, 40 insertions(+), 10 deletions(-) diff --git a/src/options.c b/src/options.c index 76cdbef0..cb5d4049 100644 --- a/src/options.c +++ b/src/options.c @@ -70,7 +70,7 @@ static mi_option_desc_t options[_mi_option_last] = { 0, UNINIT, MI_OPTION(page_reset) }, // reset page memory on free { 0, UNINIT, MI_OPTION(abandoned_page_reset) },// reset free page memory when a thread terminates { 0, UNINIT, MI_OPTION(segment_reset) }, // reset segment memory on free (needs eager commit) - { 0, UNINIT, MI_OPTION(eager_commit_delay) }, // the first N segments per thread are not eagerly committed + { 1, UNINIT, MI_OPTION(eager_commit_delay) }, // the first N segments per thread are not eagerly committed { 100, UNINIT, MI_OPTION(reset_delay) }, // reset delay in milli-seconds { 0, UNINIT, MI_OPTION(use_numa_nodes) }, // 0 = use available numa nodes, otherwise use at most N nodes. { 100, UNINIT, MI_OPTION(os_tag) }, // only apple specific for now but might serve more or less related purpose diff --git a/src/segment.c b/src/segment.c index 95ae6d8b..a4b61377 100644 --- a/src/segment.c +++ b/src/segment.c @@ -231,6 +231,7 @@ static void mi_segment_protect(mi_segment_t* segment, bool protect, mi_os_tld_t* ----------------------------------------------------------- */ static void mi_page_reset(mi_segment_t* segment, mi_page_t* page, size_t size, mi_segments_tld_t* tld) { + mi_assert_internal(page->is_committed); if (!mi_option_is_enabled(mi_option_page_reset)) return; if (segment->mem_is_fixed || page->segment_in_use || page->is_reset) return; size_t psize; @@ -330,7 +331,7 @@ static void mi_pages_reset_remove_all_in_segment(mi_segment_t* segment, bool for if (segment->mem_is_fixed) return; // never reset in huge OS pages for (size_t i = 0; i < segment->capacity; i++) { mi_page_t* page = &segment->pages[i]; - if (!page->segment_in_use && !page->is_reset) { + if (!page->segment_in_use && page->is_committed && !page->is_reset) { mi_pages_reset_remove(page, tld); if (force_reset) { mi_page_reset(segment, page, 0, tld); @@ -544,8 +545,12 @@ void _mi_segment_thread_collect(mi_segments_tld_t* tld) { } mi_assert_internal(tld->cache_count == 0); mi_assert_internal(tld->cache == NULL); - mi_assert_internal(tld->pages_reset.first == NULL); - mi_assert_internal(tld->pages_reset.last == NULL); +#if MI_DEBUG>=2 + if (!_mi_is_main_thread()) { + mi_assert_internal(tld->pages_reset.first == NULL); + mi_assert_internal(tld->pages_reset.last == NULL); + } +#endif } @@ -979,7 +984,7 @@ static bool mi_segment_pages_collect(mi_segment_t* segment, size_t block_size, m // if everything free already, clear the page directly segment->abandoned--; _mi_stat_decrease(&tld->stats->pages_abandoned, 1); - mi_segment_page_clear(segment, page, false, tld); // no reset allowed (as the segment is still abandoned) + mi_segment_page_clear(segment, page, false, tld); // no (delayed) reset allowed (as the segment is still abandoned) has_page = true; } else if (page->xblock_size == block_size && page->used < page->reserved) { @@ -987,6 +992,9 @@ static bool mi_segment_pages_collect(mi_segment_t* segment, size_t block_size, m has_page = true; } } + else { + has_page = true; + } } return has_page; } @@ -1046,7 +1054,8 @@ static mi_segment_t* mi_segment_reclaim(mi_segment_t* segment, mi_heap_t* heap, } else { // otherwise return the segment as it will contain some free pages - mi_assert_internal(segment->used < segment->capacity); + // (except for abandoned_reclaim_all which uses a block_size of zero) + mi_assert_internal(segment->used < segment->capacity || block_size == 0); return segment; } } diff --git a/test/test-stress.c b/test/test-stress.c index 67ec9f05..7869cc8c 100644 --- a/test/test-stress.c +++ b/test/test-stress.c @@ -57,6 +57,7 @@ const uintptr_t cookie = 0xbf58476d1ce4e5b9UL; const uintptr_t cookie = 0x1ce4e5b9UL; #endif +static uintptr_t ticks(void); static void* atomic_exchange_ptr(volatile void** p, void* newval); typedef uintptr_t* random_t; @@ -121,7 +122,7 @@ static void free_items(void* p) { static void stress(intptr_t tid) { //bench_start_thread(); - uintptr_t r = tid * 43; + uintptr_t r = (tid * 43)^ticks(); const size_t max_item_shift = 5; // 128 const size_t max_item_retained_shift = max_item_shift + 2; size_t allocs = 100 * ((size_t)SCALE) * (tid % 8 + 1); // some threads do more @@ -194,9 +195,9 @@ static void test_stress(void) { } static void leak(intptr_t tid) { - uintptr_t r = 43*tid; + uintptr_t r = (43*tid)^ticks(); void* p = alloc_items(pick(&r)%128, &r); - if (chance(10, &r)) { + if (chance(50, &r)) { intptr_t i = (pick(&r) % TRANSFERS); void* q = atomic_exchange_ptr(&transfer[i], p); free_items(q); @@ -259,7 +260,13 @@ static void (*thread_entry_fun)(intptr_t) = &stress; #include -static DWORD WINAPI thread_entry(LPVOID param) { +static uintptr_t ticks(void) { + LARGE_INTEGER t; + QueryPerformanceCounter(&t); + return (uintptr_t)t.QuadPart; +} + +static DWORD WINAPI thread_entry(LPVOID param) { thread_entry_fun((intptr_t)param); return 0; } @@ -323,4 +330,18 @@ static void* atomic_exchange_ptr(volatile void** p, void* newval) { } #endif +#include +#ifdef CLOCK_REALTIME +uintptr_t ticks(void) { + struct timespec t; + clock_gettime(CLOCK_REALTIME, &t); + return (uintptr_t)t.tv_sec * 1000) + ((uintptr_t)t.tv_nsec / 1000000); +} +#else +// low resolution timer +uintptr_t _mi_clock_now(void) { + return ((uintptr_t)clock() / ((uintptr_t)CLOCKS_PER_SEC / 1000)); +} +#endif + #endif From e68293741edb043e9e8bdbfa06896d5c187024f7 Mon Sep 17 00:00:00 2001 From: daan Date: Thu, 23 Jan 2020 21:44:32 -0800 Subject: [PATCH 11/28] fix assertion, add check for page committed before doing reset --- src/segment.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/segment.c b/src/segment.c index 85e8817b..3914d770 100644 --- a/src/segment.c +++ b/src/segment.c @@ -231,6 +231,7 @@ static void mi_segment_protect(mi_segment_t* segment, bool protect, mi_os_tld_t* ----------------------------------------------------------- */ static void mi_page_reset(mi_segment_t* segment, mi_page_t* page, size_t size, mi_segments_tld_t* tld) { + mi_assert_internal(page->is_committed); if (!mi_option_is_enabled(mi_option_page_reset)) return; if (segment->mem_is_fixed || page->segment_in_use || page->is_reset) return; size_t psize; @@ -330,7 +331,7 @@ static void mi_pages_reset_remove_all_in_segment(mi_segment_t* segment, bool for if (segment->mem_is_fixed) return; // never reset in huge OS pages for (size_t i = 0; i < segment->capacity; i++) { mi_page_t* page = &segment->pages[i]; - if (!page->segment_in_use && !page->is_reset) { + if (!page->segment_in_use && page->is_committed && !page->is_reset) { mi_pages_reset_remove(page, tld); if (force_reset) { mi_page_reset(segment, page, 0, tld); @@ -544,8 +545,12 @@ void _mi_segment_thread_collect(mi_segments_tld_t* tld) { } mi_assert_internal(tld->cache_count == 0); mi_assert_internal(tld->cache == NULL); - mi_assert_internal(tld->pages_reset.first == NULL); - mi_assert_internal(tld->pages_reset.last == NULL); +#if MI_DEBUG>=2 + if (!_mi_is_main_thread()) { + mi_assert_internal(tld->pages_reset.first == NULL); + mi_assert_internal(tld->pages_reset.last == NULL); + } +#endif } From 8cf4882a85f9ab64c77bc93898b71aedf27a1dbb Mon Sep 17 00:00:00 2001 From: daan Date: Fri, 24 Jan 2020 10:38:25 -0800 Subject: [PATCH 12/28] fix linux build --- test/test-stress.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test-stress.c b/test/test-stress.c index 6e2b20e9..40ddbd47 100644 --- a/test/test-stress.c +++ b/test/test-stress.c @@ -335,7 +335,7 @@ static void* atomic_exchange_ptr(volatile void** p, void* newval) { uintptr_t ticks(void) { struct timespec t; clock_gettime(CLOCK_REALTIME, &t); - return (uintptr_t)t.tv_sec * 1000) + ((uintptr_t)t.tv_nsec / 1000000); + return ((uintptr_t)t.tv_sec * 1000) + ((uintptr_t)t.tv_nsec / 1000000); } #else // low resolution timer From 28c14d99c317063fc6a869a9261f125a30106fe5 Mon Sep 17 00:00:00 2001 From: daan Date: Fri, 24 Jan 2020 11:03:12 -0800 Subject: [PATCH 13/28] clean up comments --- src/memory.c | 59 +++++++++++++++++++++++++++------------------------- 1 file changed, 31 insertions(+), 28 deletions(-) diff --git a/src/memory.c b/src/memory.c index a442a35d..287de414 100644 --- a/src/memory.c +++ b/src/memory.c @@ -57,7 +57,7 @@ void* _mi_arena_alloc_aligned(size_t size, size_t alignment, bool* commit, boo // Constants #if (MI_INTPTR_SIZE==8) -#define MI_HEAP_REGION_MAX_SIZE (256 * GiB) // 48KiB for the region map +#define MI_HEAP_REGION_MAX_SIZE (256 * GiB) // 64KiB for the region map #elif (MI_INTPTR_SIZE==4) #define MI_HEAP_REGION_MAX_SIZE (3 * GiB) // ~ KiB for the region map #else @@ -72,14 +72,13 @@ void* _mi_arena_alloc_aligned(size_t size, size_t alignment, bool* commit, boo #define MI_REGION_MAX_OBJ_BLOCKS (MI_REGION_MAX_BLOCKS/4) // 64MiB #define MI_REGION_MAX_OBJ_SIZE (MI_REGION_MAX_OBJ_BLOCKS*MI_SEGMENT_SIZE) -// Region info is a pointer to the memory region and two bits for -// its flags: is_large, and is_committed. +// Region info typedef union mi_region_info_u { - uintptr_t value; + uintptr_t value; struct { - bool valid; - bool is_large; - short numa_node; + bool valid; // initialized? + bool is_large; // allocated in fixed large/huge OS pages + short numa_node; // the associated NUMA node (where -1 means no associated node) } x; } mi_region_info_t; @@ -87,12 +86,12 @@ typedef union mi_region_info_u { // A region owns a chunk of REGION_SIZE (256MiB) (virtual) memory with // a bit map with one bit per MI_SEGMENT_SIZE (4MiB) block. typedef struct mem_region_s { - volatile _Atomic(uintptr_t) info; // is_large, and associated numa node + 1 (so 0 is no association) - volatile _Atomic(void*) start; // start of the memory area (and flags) + volatile _Atomic(uintptr_t) info; // mi_region_info_t.value + volatile _Atomic(void*) start; // start of the memory area mi_bitmap_field_t in_use; // bit per in-use block mi_bitmap_field_t dirty; // track if non-zero per block - mi_bitmap_field_t commit; // track if committed per block (if `!info.is_committed)) - mi_bitmap_field_t reset; // track reset per block + mi_bitmap_field_t commit; // track if committed per block + mi_bitmap_field_t reset; // track if reset per block volatile _Atomic(uintptr_t) arena_memid; // if allocated from a (huge page) arena- } mem_region_t; @@ -239,11 +238,13 @@ static bool mi_region_try_claim(int numa_node, size_t blocks, bool allow_large, { // try all regions for a free slot const size_t count = mi_atomic_read(®ions_count); - size_t idx = tld->region_idx; // Or start at 0 to reuse low addresses? + size_t idx = tld->region_idx; // Or start at 0 to reuse low addresses? Starting at 0 seems to increase latency though for (size_t visited = 0; visited < count; visited++, idx++) { if (idx >= count) idx = 0; // wrap around mem_region_t* r = ®ions[idx]; + // if this region suits our demand (numa node matches, large OS page matches) if (mi_region_is_suitable(r, numa_node, allow_large)) { + // then try to atomically claim a segment(s) in this region if (mi_bitmap_try_find_claim_field(&r->in_use, 0, blocks, bit_idx)) { tld->region_idx = idx; // remember the last found position *region = r; @@ -263,15 +264,15 @@ static void* mi_region_try_alloc(size_t blocks, bool* commit, bool* is_large, bo const int numa_node = (_mi_os_numa_node_count() <= 1 ? -1 : _mi_os_numa_node(tld)); // try to claim in existing regions if (!mi_region_try_claim(numa_node, blocks, *is_large, ®ion, &bit_idx, tld)) { - // otherwise try to allocate a fresh region + // otherwise try to allocate a fresh region and claim in there if (!mi_region_try_alloc_os(blocks, *commit, *is_large, ®ion, &bit_idx, tld)) { // out of regions or memory return NULL; } } - - // found a region and claimed `blocks` at `bit_idx` + // ------------------------------------------------ + // found a region and claimed `blocks` at `bit_idx`, initialize them now mi_assert_internal(region != NULL); mi_assert_internal(mi_bitmap_is_claimed(®ion->in_use, 1, blocks, bit_idx)); @@ -346,25 +347,27 @@ void* _mi_mem_alloc_aligned(size_t size, size_t alignment, bool* commit, bool* l size = _mi_align_up(size, _mi_os_page_size()); // allocate from regions if possible + void* p = NULL; size_t arena_memid; const size_t blocks = mi_region_block_count(size); if (blocks <= MI_REGION_MAX_OBJ_BLOCKS && alignment <= MI_SEGMENT_ALIGN) { - void* p = mi_region_try_alloc(blocks, commit, large, is_zero, memid, tld); - mi_assert_internal(p == NULL || (uintptr_t)p % alignment == 0); - if (p != NULL) { - #if (MI_DEBUG>=2) - if (*commit) { ((uint8_t*)p)[0] = 0; } - #endif - return p; + p = mi_region_try_alloc(blocks, commit, large, is_zero, memid, tld); + if (p == NULL) { + _mi_warning_message("unable to allocate from region: size %zu\n", size); } - _mi_warning_message("unable to allocate from region: size %zu\n", size); + } + if (p == NULL) { + // and otherwise fall back to the OS + p = _mi_arena_alloc_aligned(size, alignment, commit, large, is_zero, &arena_memid, tld); + *memid = mi_memid_create_from_arena(arena_memid); } - // and otherwise fall back to the OS - void* p = _mi_arena_alloc_aligned(size, alignment, commit, large, is_zero, &arena_memid, tld); - *memid = mi_memid_create_from_arena(arena_memid); - mi_assert_internal( p == NULL || (uintptr_t)p % alignment == 0); - if (p != NULL && *commit) { ((uint8_t*)p)[0] = 0; } + if (p != NULL) { + mi_assert_internal((uintptr_t)p % alignment == 0); +#if (MI_DEBUG>=2) + if (*commit) { ((uint8_t*)p)[0] = 0; } // ensure the memory is committed +#endif + } return p; } From 4ae51096ecdee7f1d4b309f38f6c272a8f61d473 Mon Sep 17 00:00:00 2001 From: daan Date: Fri, 24 Jan 2020 15:45:03 -0800 Subject: [PATCH 14/28] add warning on region exhaustion --- src/memory.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/memory.c b/src/memory.c index 287de414..96047b79 100644 --- a/src/memory.c +++ b/src/memory.c @@ -92,7 +92,8 @@ typedef struct mem_region_s { mi_bitmap_field_t dirty; // track if non-zero per block mi_bitmap_field_t commit; // track if committed per block mi_bitmap_field_t reset; // track if reset per block - volatile _Atomic(uintptr_t) arena_memid; // if allocated from a (huge page) arena- + volatile _Atomic(uintptr_t) arena_memid; // if allocated from a (huge page) arena + uintptr_t padding; // round to 8 fields } mem_region_t; // The region map @@ -187,6 +188,7 @@ static bool mi_region_try_alloc_os(size_t blocks, bool commit, bool allow_large, if (idx >= MI_REGION_MAX) { mi_atomic_decrement(®ions_count); _mi_arena_free(start, MI_REGION_SIZE, arena_memid, tld->stats); + _mi_warning_message("maximum regions used: %zu GiB (perhaps recompile with a larger setting for MI_HEAP_REGION_MAX_SIZE)", _mi_divide_up(MI_HEAP_REGION_MAX_SIZE, GiB)); return false; } From e070eba112f80b4b4c007cc8cd6696463bf1884b Mon Sep 17 00:00:00 2001 From: daan Date: Fri, 24 Jan 2020 16:30:52 -0800 Subject: [PATCH 15/28] fix tagged null encoding, search segment cache before reclaim --- src/options.c | 2 +- src/segment.c | 149 +++++++++++++++++++++++++++++++-------------- test/test-stress.c | 16 ++--- 3 files changed, 111 insertions(+), 56 deletions(-) diff --git a/src/options.c b/src/options.c index cb5d4049..af051aa2 100644 --- a/src/options.c +++ b/src/options.c @@ -67,7 +67,7 @@ static mi_option_desc_t options[_mi_option_last] = { 0, UNINIT, MI_OPTION(large_os_pages) }, // use large OS pages, use only with eager commit to prevent fragmentation of VMA's { 0, UNINIT, MI_OPTION(reserve_huge_os_pages) }, { 0, UNINIT, MI_OPTION(segment_cache) }, // cache N segments per thread - { 0, UNINIT, MI_OPTION(page_reset) }, // reset page memory on free + { 1, UNINIT, MI_OPTION(page_reset) }, // reset page memory on free { 0, UNINIT, MI_OPTION(abandoned_page_reset) },// reset free page memory when a thread terminates { 0, UNINIT, MI_OPTION(segment_reset) }, // reset segment memory on free (needs eager commit) { 1, UNINIT, MI_OPTION(eager_commit_delay) }, // the first N segments per thread are not eagerly committed diff --git a/src/segment.c b/src/segment.c index a4b61377..7aced87d 100644 --- a/src/segment.c +++ b/src/segment.c @@ -15,27 +15,25 @@ terms of the MIT license. A copy of the license can be found in the file static uint8_t* mi_segment_raw_page_start(const mi_segment_t* segment, const mi_page_t* page, size_t* page_size); -/* ----------------------------------------------------------- +/* -------------------------------------------------------------------------------- Segment allocation - We allocate pages inside big OS allocated "segments" - (4mb on 64-bit). This is to avoid splitting VMA's on Linux - and reduce fragmentation on other OS's. Each thread - owns its own segments. + We allocate pages inside bigger "segments" (4mb on 64-bit). This is to avoid + splitting VMA's on Linux and reduce fragmentation on other OS's. + Each thread owns its own segments. Currently we have: - small pages (64kb), 64 in one segment - medium pages (512kb), 8 in one segment - large pages (4mb), 1 in one segment - - huge blocks > MI_LARGE_OBJ_SIZE_MAX (512kb) are directly allocated by the OS + - huge blocks > MI_LARGE_OBJ_SIZE_MAX become large segment with 1 page - In any case the memory for a segment is virtual and only - committed on demand (i.e. we are careful to not touch the memory - until we actually allocate a block there) + In any case the memory for a segment is virtual and usually committed on demand. + (i.e. we are careful to not touch the memory until we actually allocate a block there) If a thread ends, it "abandons" pages with used blocks and there is an abandoned segment list whose segments can be reclaimed by still running threads, much like work-stealing. ------------------------------------------------------------ */ +-------------------------------------------------------------------------------- */ /* ----------------------------------------------------------- @@ -559,8 +557,11 @@ void _mi_segment_thread_collect(mi_segments_tld_t* tld) { ----------------------------------------------------------- */ // Allocate a segment from the OS aligned to `MI_SEGMENT_SIZE` . -static mi_segment_t* mi_segment_alloc(size_t required, mi_page_kind_t page_kind, size_t page_shift, mi_segments_tld_t* tld, mi_os_tld_t* os_tld) +static mi_segment_t* mi_segment_init(mi_segment_t* segment, size_t required, mi_page_kind_t page_kind, size_t page_shift, mi_segments_tld_t* tld, mi_os_tld_t* os_tld) { + // the segment parameter is non-null if it came from our cache + mi_assert_internal(segment==NULL || (required==0 && page_kind <= MI_PAGE_LARGE)); + // calculate needed sizes first size_t capacity; if (page_kind == MI_PAGE_HUGE) { @@ -587,8 +588,9 @@ static mi_segment_t* mi_segment_alloc(size_t required, mi_page_kind_t page_kind, bool is_zero = false; // Try to get it from our thread local cache first - mi_segment_t* segment = mi_segment_cache_pop(segment_size, tld); if (segment != NULL) { + // came from cache + mi_assert_internal(segment->segment_size == segment_size); if (page_kind <= MI_PAGE_MEDIUM && segment->page_kind == page_kind && segment->segment_size == segment_size) { pages_still_good = true; } @@ -674,6 +676,9 @@ static mi_segment_t* mi_segment_alloc(size_t required, mi_page_kind_t page_kind, return segment; } +static mi_segment_t* mi_segment_alloc(size_t required, mi_page_kind_t page_kind, size_t page_shift, mi_segments_tld_t* tld, mi_os_tld_t* os_tld) { + return mi_segment_init(NULL, required, page_kind, page_shift, tld, os_tld); +} static void mi_segment_free(mi_segment_t* segment, bool force, mi_segments_tld_t* tld) { UNUSED(force); @@ -814,15 +819,23 @@ void _mi_segment_page_free(mi_page_t* page, bool force, mi_segments_tld_t* tld) /* ----------------------------------------------------------- - Abandonment +Abandonment + +When threads terminate, they can leave segments with +live blocks (reached through other threads). Such segments +are "abandoned" and will be reclaimed by other threads to +reuse their pages and/or free them eventually + +We maintain a global list of abandoned segments that are +reclaimed on demand. Since this is shared among threads +the implementation needs to avoid the A-B-A problem on +popping abandoned segments which is why tagged pointers are +used. ----------------------------------------------------------- */ -// When threads terminate, they can leave segments with -// live blocks (reached through other threads). Such segments -// are "abandoned" and will be reclaimed by other threads to -// reuse their pages and/or free them eventually - - +// Use the bottom 20-bits (on 64-bit) of the aligned segment +// pointers to put in a tag that increments on update to avoid +// the A-B-A problem. #define MI_TAGGED_MASK MI_SEGMENT_MASK typedef uintptr_t mi_tagged_segment_t; @@ -832,15 +845,23 @@ static mi_segment_t* mi_tagged_segment_ptr(mi_tagged_segment_t ts) { static mi_tagged_segment_t mi_tagged_segment(mi_segment_t* segment, mi_tagged_segment_t ts) { mi_assert_internal(((uintptr_t)segment & MI_TAGGED_MASK) == 0); - if (segment==NULL) return 0; // no need to tag NULL uintptr_t tag = ((ts & MI_TAGGED_MASK) + 1) & MI_TAGGED_MASK; return ((uintptr_t)segment | tag); } +// This is a list of visited abandoned pages that were full at the time. +// this list migrates to `abandoned` when that becomes NULL. static volatile _Atomic(mi_segment_t*) abandoned_visited; // = NULL + +// The abandoned page list. static volatile _Atomic(mi_tagged_segment_t) abandoned; // = NULL + +// We also maintain a count of current readers of the abandoned list +// in order to prevent resetting/decommitting segment memory if it might +// still be read. static volatile _Atomic(uintptr_t) abandoned_readers; // = 0 +// Push on the visited list static void mi_abandoned_visited_push(mi_segment_t* segment) { mi_assert_internal(segment->thread_id == 0); mi_assert_internal(segment->abandoned_next == NULL); @@ -853,14 +874,23 @@ static void mi_abandoned_visited_push(mi_segment_t* segment) { } while (!mi_atomic_cas_ptr_weak(mi_segment_t, &abandoned_visited, segment, anext)); } -static bool mi_abandoned_visited_revisit(void) { +// Move the visited list to the abandoned list. +static bool mi_abandoned_visited_revisit(void) +{ + // quick check if the visited list is empty + if (mi_atomic_read_ptr_relaxed(mi_segment_t,&abandoned_visited)==NULL) return false; + // grab the whole visited list mi_segment_t* first = mi_atomic_exchange_ptr(mi_segment_t, &abandoned_visited, NULL); if (first == NULL) return false; // first try to swap directly if the abandoned list happens to be NULL - mi_tagged_segment_t afirst = mi_tagged_segment(first,0); - if (mi_atomic_cas_weak(&abandoned, afirst, 0)) return true; + const mi_tagged_segment_t ts = mi_atomic_read_relaxed(&abandoned); + mi_tagged_segment_t afirst; + if (mi_tagged_segment_ptr(ts)==NULL) { + afirst = mi_tagged_segment(first, ts); + if (mi_atomic_cas_strong(&abandoned, afirst, ts)) return true; + } // find the last element of the visited list: O(n) mi_segment_t* last = first; @@ -879,6 +909,7 @@ static bool mi_abandoned_visited_revisit(void) { return true; } +// Push on the abandoned list. static void mi_abandoned_push(mi_segment_t* segment) { mi_assert_internal(segment->thread_id == 0); mi_assert_internal(segment->abandoned_next == NULL); @@ -893,6 +924,7 @@ static void mi_abandoned_push(mi_segment_t* segment) { } while (!mi_atomic_cas_weak(&abandoned, next, ts)); } +// Wait until there are no more pending reads on segments that used to be in the abandoned list void _mi_abandoned_await_readers(void) { uintptr_t n; do { @@ -901,23 +933,28 @@ void _mi_abandoned_await_readers(void) { } while (n != 0); } +// Pop from the abandoned list static mi_segment_t* mi_abandoned_pop(void) { mi_segment_t* segment; + // Check efficiently if it is empty (or if the visited list needs to be moved) mi_tagged_segment_t ts = mi_atomic_read_relaxed(&abandoned); segment = mi_tagged_segment_ptr(ts); - if (segment == NULL) { - if (!mi_abandoned_visited_revisit()) return NULL; // try to swap in the visited list on NULL + if (mi_likely(segment == NULL)) { + if (mi_likely(!mi_abandoned_visited_revisit())) { // try to swap in the visited list on NULL + return NULL; + } } - // Do a pop. We use a reader lock to prevent - // a segment to be decommitted while a read is still pending, and a tagged - // pointer to prevent A-B-A link corruption. + + // Do a pop. We use a reader count to prevent + // a segment to be decommitted while a read is still pending, + // and a tagged pointer to prevent A-B-A link corruption. mi_atomic_increment(&abandoned_readers); // ensure no segment gets decommitted mi_tagged_segment_t next = 0; do { ts = mi_atomic_read_relaxed(&abandoned); segment = mi_tagged_segment_ptr(ts); if (segment != NULL) { - next = mi_tagged_segment(segment->abandoned_next, ts); // note: reads segment so should not be decommitted + next = mi_tagged_segment(segment->abandoned_next, ts); // note: reads the segment's `abandoned_next` field so should not be decommitted } } while (segment != NULL && !mi_atomic_cas_weak(&abandoned, next, ts)); mi_atomic_decrement(&abandoned_readers); // release reader lock @@ -927,6 +964,9 @@ static mi_segment_t* mi_abandoned_pop(void) { return segment; } +/* ----------------------------------------------------------- + Abandon segment/page +----------------------------------------------------------- */ static void mi_segment_abandon(mi_segment_t* segment, mi_segments_tld_t* tld) { mi_assert_internal(segment->used == segment->abandoned); @@ -945,7 +985,7 @@ static void mi_segment_abandon(mi_segment_t* segment, mi_segments_tld_t* tld) { mi_segments_track_size(-((long)segment->segment_size), tld); segment->thread_id = 0; segment->abandoned_next = NULL; - mi_abandoned_push(segment); + mi_abandoned_push(segment); } void _mi_segment_page_abandon(mi_page_t* page, mi_segments_tld_t* tld) { @@ -964,6 +1004,9 @@ void _mi_segment_page_abandon(mi_page_t* page, mi_segments_tld_t* tld) { } } +/* ----------------------------------------------------------- + Reclaim abandoned pages +----------------------------------------------------------- */ static bool mi_segment_pages_collect(mi_segment_t* segment, size_t block_size, mi_segments_tld_t* tld) { @@ -1082,22 +1125,6 @@ static mi_segment_t* mi_segment_try_reclaim(mi_heap_t* heap, size_t block_size, return NULL; } -static mi_segment_t* mi_segment_reclaim_or_alloc(mi_heap_t* heap, size_t block_size, mi_page_kind_t page_kind, size_t page_shift, mi_segments_tld_t* tld, mi_os_tld_t* os_tld) -{ - mi_assert_internal(page_kind <= MI_PAGE_LARGE); - mi_assert_internal(block_size < MI_HUGE_BLOCK_SIZE); - mi_segment_t* segment = mi_segment_try_reclaim(heap, block_size, page_kind, tld); - if (segment == MI_RECLAIMED) { - return NULL; // pretend out-of-memory as the page will be in the page queue - } - else if (segment == NULL) { - return mi_segment_alloc(0, page_kind, page_shift, tld, os_tld); - } - else { - return segment; - } -} - void _mi_abandoned_reclaim_all(mi_heap_t* heap, mi_segments_tld_t* tld) { mi_segment_t* segment; while ((segment = mi_abandoned_pop()) != NULL) { @@ -1112,6 +1139,34 @@ void _mi_abandoned_reclaim_all(mi_heap_t* heap, mi_segments_tld_t* tld) { } } + +/* ----------------------------------------------------------- + Reclaim or allocate +----------------------------------------------------------- */ + +static mi_segment_t* mi_segment_reclaim_or_alloc(mi_heap_t* heap, size_t block_size, mi_page_kind_t page_kind, size_t page_shift, mi_segments_tld_t* tld, mi_os_tld_t* os_tld) +{ + mi_assert_internal(page_kind <= MI_PAGE_LARGE); + mi_assert_internal(block_size < MI_HUGE_BLOCK_SIZE); + // 1. try to get a segment from our cache + mi_segment_t* segment = mi_segment_cache_pop(MI_SEGMENT_SIZE, tld); + if (segment != NULL) { + mi_segment_init(segment, 0, page_kind, page_shift, tld, os_tld); + return segment; + } + // 2. try to reclaim an abandoned segment + segment = mi_segment_try_reclaim(heap, block_size, page_kind, tld); + if (segment == MI_RECLAIMED) { + return NULL; // pretend out-of-memory as the page will be in the page queue of the heap + } + else if (segment != NULL) { + return segment; // reclaimed a segment with empty pages in it + } + // 3. otherwise allocate a fresh segment + return mi_segment_alloc(0, page_kind, page_shift, tld, os_tld); +} + + /* ----------------------------------------------------------- Small page allocation ----------------------------------------------------------- */ @@ -1192,7 +1247,7 @@ static mi_page_t* mi_segment_huge_page_alloc(size_t size, mi_segments_tld_t* tld } /* ----------------------------------------------------------- - Page allocation and free + Page allocation ----------------------------------------------------------- */ mi_page_t* _mi_segment_page_alloc(mi_heap_t* heap, size_t block_size, mi_segments_tld_t* tld, mi_os_tld_t* os_tld) { diff --git a/test/test-stress.c b/test/test-stress.c index 40ddbd47..72e4e853 100644 --- a/test/test-stress.c +++ b/test/test-stress.c @@ -32,8 +32,10 @@ static int ITER = 50; // N full iterations destructing and re-creating a // static int THREADS = 8; // more repeatable if THREADS <= #processors // static int SCALE = 100; // scaling factor +#define STRESS // undefine for leak test + static bool allow_large_objects = true; // allow very large objects? -static size_t use_one_size = 0; // use single object size of `N * sizeof(uintptr_t)`? +static size_t use_one_size = 1; // use single object size of `N * sizeof(uintptr_t)`? #ifdef USE_STD_MALLOC @@ -189,7 +191,7 @@ static void test_stress(void) { } mi_collect(false); #ifndef NDEBUG - if ((n + 1) % 10 == 0) { printf("- iterations left: %3d\n", ITER - (n + 1)); } + if ((n + 1) % 10 == 0) { printf("- iterations left: %3d\n", ITER - (n + 1)); } #endif } } @@ -209,8 +211,7 @@ static void test_leak(void) { run_os_threads(THREADS, &leak); mi_collect(false); #ifndef NDEBUG - //if ((n + 1) % 10 == 0) - { printf("- iterations left: %3d\n", ITER - (n + 1)); } + if ((n + 1) % 10 == 0) { printf("- iterations left: %3d\n", ITER - (n + 1)); } #endif } } @@ -240,12 +241,11 @@ int main(int argc, char** argv) { // Run ITER full iterations where half the objects in the transfer buffer survive to the next round. mi_stats_reset(); - if (true) { +#ifdef STRESS test_stress(); - } - else { +#else test_leak(); - } +#endif mi_collect(true); mi_stats_print(NULL); From b31bc52618658bdb12cb316c13580ecd82bfd8d9 Mon Sep 17 00:00:00 2001 From: daan Date: Fri, 24 Jan 2020 19:02:13 -0800 Subject: [PATCH 16/28] add cache alignment directives for contended variables --- ide/vs2019/mimalloc.vcxproj | 4 ++-- include/mimalloc-internal.h | 4 ++++ src/arena.c | 6 +++--- src/os.c | 4 ++-- src/segment.c | 26 +++++++++++++------------- test/test-stress.c | 6 +++--- 6 files changed, 27 insertions(+), 23 deletions(-) diff --git a/ide/vs2019/mimalloc.vcxproj b/ide/vs2019/mimalloc.vcxproj index 037e380d..a98e78ba 100644 --- a/ide/vs2019/mimalloc.vcxproj +++ b/ide/vs2019/mimalloc.vcxproj @@ -100,7 +100,7 @@ MI_DEBUG=3;%(PreprocessorDefinitions); CompileAsCpp false - stdcpp17 + Default @@ -119,7 +119,7 @@ MI_DEBUG=3;%(PreprocessorDefinitions); CompileAsCpp false - stdcpp17 + Default diff --git a/include/mimalloc-internal.h b/include/mimalloc-internal.h index 3335414a..902d2fdf 100644 --- a/include/mimalloc-internal.h +++ b/include/mimalloc-internal.h @@ -20,16 +20,20 @@ terms of the MIT license. A copy of the license can be found in the file #define mi_trace_message(...) #endif +#define MI_CACHE_LINE 64 #if defined(_MSC_VER) #pragma warning(disable:4127) // suppress constant conditional warning (due to MI_SECURE paths) #define mi_decl_noinline __declspec(noinline) #define mi_decl_thread __declspec(thread) +#define mi_decl_cache_align __declspec(align(MI_CACHE_LINE)) #elif (defined(__GNUC__) && (__GNUC__>=3)) // includes clang and icc #define mi_decl_noinline __attribute__((noinline)) #define mi_decl_thread __thread +#define mi_decl_cache_align __attribute__((aligned(MI_CACHE_LINE))) #else #define mi_decl_noinline #define mi_decl_thread __thread // hope for the best :-) +#define mi_decl_cache_align #endif diff --git a/src/arena.c b/src/arena.c index acb92243..ac599f32 100644 --- a/src/arena.c +++ b/src/arena.c @@ -54,7 +54,7 @@ bool _mi_os_commit(void* p, size_t size, bool* is_zero, mi_stats_t* stats); #define MI_MAX_ARENAS (64) // not more than 256 (since we use 8 bits in the memid) // A memory arena descriptor -typedef struct mi_arena_s { +typedef mi_decl_cache_align struct mi_arena_s { _Atomic(uint8_t*) start; // the start of the memory area size_t block_count; // size of the area in arena blocks (of `MI_ARENA_BLOCK_SIZE`) size_t field_count; // number of bitmap fields (where `field_count * MI_BITMAP_FIELD_BITS >= block_count`) @@ -70,8 +70,8 @@ typedef struct mi_arena_s { // The available arenas -static _Atomic(mi_arena_t*) mi_arenas[MI_MAX_ARENAS]; -static _Atomic(uintptr_t) mi_arena_count; // = 0 +static mi_decl_cache_align _Atomic(mi_arena_t*) mi_arenas[MI_MAX_ARENAS]; +static mi_decl_cache_align _Atomic(uintptr_t) mi_arena_count; // = 0 /* ----------------------------------------------------------- diff --git a/src/os.c b/src/os.c index 6e8c12d8..b8dfaa70 100644 --- a/src/os.c +++ b/src/os.c @@ -397,7 +397,7 @@ static void* mi_unix_mmap(void* addr, size_t size, size_t try_alignment, int pro // On 64-bit systems, we can do efficient aligned allocation by using // the 4TiB to 30TiB area to allocate them. #if (MI_INTPTR_SIZE >= 8) && (defined(_WIN32) || (defined(MI_OS_USE_MMAP) && !defined(MAP_ALIGNED))) -static volatile _Atomic(uintptr_t) aligned_base; +static volatile mi_decl_cache_align _Atomic(uintptr_t) aligned_base; // Return a 4MiB aligned address that is probably available static void* mi_os_get_aligned_hint(size_t try_alignment, size_t size) { @@ -905,7 +905,7 @@ static void* mi_os_alloc_huge_os_pagesx(void* addr, size_t size, int numa_node) #if (MI_INTPTR_SIZE >= 8) // To ensure proper alignment, use our own area for huge OS pages -static _Atomic(uintptr_t) mi_huge_start; // = 0 +static mi_decl_cache_align _Atomic(uintptr_t) mi_huge_start; // = 0 // Claim an aligned address range for huge pages static uint8_t* mi_os_claim_huge_pages(size_t pages, size_t* total_size) { diff --git a/src/segment.c b/src/segment.c index 7aced87d..a26ac449 100644 --- a/src/segment.c +++ b/src/segment.c @@ -365,9 +365,6 @@ static void mi_reset_delayed(mi_segments_tld_t* tld) { } - - - /* ----------------------------------------------------------- Segment size calculations ----------------------------------------------------------- */ @@ -829,13 +826,15 @@ reuse their pages and/or free them eventually We maintain a global list of abandoned segments that are reclaimed on demand. Since this is shared among threads the implementation needs to avoid the A-B-A problem on -popping abandoned segments which is why tagged pointers are -used. +popping abandoned segments: +We use tagged pointers to avoid accidentially identifying +reused segments, much like stamped references in Java. +Secondly, we maintain a reader counter to avoid resetting +or decommitting segments that have a pending read operation. ----------------------------------------------------------- */ -// Use the bottom 20-bits (on 64-bit) of the aligned segment -// pointers to put in a tag that increments on update to avoid -// the A-B-A problem. +// Use the bottom 20-bits (on 64-bit) of the aligned segment pointers +// to put in a tag that increments on update to avoid the A-B-A problem. #define MI_TAGGED_MASK MI_SEGMENT_MASK typedef uintptr_t mi_tagged_segment_t; @@ -850,16 +849,17 @@ static mi_tagged_segment_t mi_tagged_segment(mi_segment_t* segment, mi_tagged_se } // This is a list of visited abandoned pages that were full at the time. -// this list migrates to `abandoned` when that becomes NULL. -static volatile _Atomic(mi_segment_t*) abandoned_visited; // = NULL +// this list migrates to `abandoned` when that becomes NULL. The use of +// this list reduces contention and the rate at which segments are visited. +static mi_decl_cache_align volatile _Atomic(mi_segment_t*) abandoned_visited; // = NULL -// The abandoned page list. -static volatile _Atomic(mi_tagged_segment_t) abandoned; // = NULL +// The abandoned page list (tagged as it supports pop) +static mi_decl_cache_align volatile _Atomic(mi_tagged_segment_t) abandoned; // = NULL // We also maintain a count of current readers of the abandoned list // in order to prevent resetting/decommitting segment memory if it might // still be read. -static volatile _Atomic(uintptr_t) abandoned_readers; // = 0 +static mi_decl_cache_align volatile _Atomic(uintptr_t) abandoned_readers; // = 0 // Push on the visited list static void mi_abandoned_visited_push(mi_segment_t* segment) { diff --git a/test/test-stress.c b/test/test-stress.c index 72e4e853..19f10360 100644 --- a/test/test-stress.c +++ b/test/test-stress.c @@ -32,10 +32,10 @@ static int ITER = 50; // N full iterations destructing and re-creating a // static int THREADS = 8; // more repeatable if THREADS <= #processors // static int SCALE = 100; // scaling factor -#define STRESS // undefine for leak test +// #define STRESS // undefine for leak test static bool allow_large_objects = true; // allow very large objects? -static size_t use_one_size = 1; // use single object size of `N * sizeof(uintptr_t)`? +static size_t use_one_size = 0; // use single object size of `N * sizeof(uintptr_t)`? #ifdef USE_STD_MALLOC @@ -198,7 +198,7 @@ static void test_stress(void) { static void leak(intptr_t tid) { uintptr_t r = (43*tid)^ticks(); - void* p = alloc_items(pick(&r)%128, &r); + void* p = alloc_items(1 /*pick(&r)%128*/, &r); if (chance(50, &r)) { intptr_t i = (pick(&r) % TRANSFERS); void* q = atomic_exchange_ptr(&transfer[i], p); From 47300eeda3e78a909492f67f7c2b77289a7be383 Mon Sep 17 00:00:00 2001 From: daan Date: Fri, 24 Jan 2020 20:17:33 -0800 Subject: [PATCH 17/28] avoid memset --- src/init.c | 7 ++++--- src/segment.c | 1 + 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/init.c b/src/init.c index 18a18f60..366acbf5 100644 --- a/src/init.c +++ b/src/init.c @@ -140,6 +140,7 @@ mi_stats_t _mi_stats_main = { MI_STATS_NULL }; Initialization and freeing of the thread local heaps ----------------------------------------------------------- */ +// note: in x64 in release build `sizeof(mi_thread_data_t)` is under 4KiB (= OS page size). typedef struct mi_thread_data_s { mi_heap_t heap; // must come first due to cast in `_mi_heap_done` mi_tld_t tld; @@ -154,12 +155,13 @@ static bool _mi_heap_init(void) { mi_assert_internal(_mi_heap_default->tld->heap_backing == mi_get_default_heap()); } else { - // use `_mi_os_alloc` to allocate directly from the OS + // use `_mi_os_alloc` to allocate directly from the OS mi_thread_data_t* td = (mi_thread_data_t*)_mi_os_alloc(sizeof(mi_thread_data_t),&_mi_stats_main); // Todo: more efficient allocation? if (td == NULL) { _mi_error_message(ENOMEM, "failed to allocate thread local heap memory\n"); return false; } + // OS allocated so already zero initialized mi_tld_t* tld = &td->tld; mi_heap_t* heap = &td->heap; memcpy(heap, &_mi_heap_empty, sizeof(*heap)); @@ -168,8 +170,7 @@ static bool _mi_heap_init(void) { heap->cookie = _mi_heap_random_next(heap) | 1; heap->key[0] = _mi_heap_random_next(heap); heap->key[1] = _mi_heap_random_next(heap); - heap->tld = tld; - memset(tld, 0, sizeof(*tld)); + heap->tld = tld; tld->heap_backing = heap; tld->segments.stats = &tld->stats; tld->segments.os = &tld->os; diff --git a/src/segment.c b/src/segment.c index a26ac449..f6554520 100644 --- a/src/segment.c +++ b/src/segment.c @@ -948,6 +948,7 @@ static mi_segment_t* mi_abandoned_pop(void) { // Do a pop. We use a reader count to prevent // a segment to be decommitted while a read is still pending, // and a tagged pointer to prevent A-B-A link corruption. + // (this is called from `memory.c:_mi_mem_free` for example) mi_atomic_increment(&abandoned_readers); // ensure no segment gets decommitted mi_tagged_segment_t next = 0; do { From ecece572847f70553f2a2c8f9d754e1f16756986 Mon Sep 17 00:00:00 2001 From: daan Date: Fri, 24 Jan 2020 20:20:43 -0800 Subject: [PATCH 18/28] fix bug in committed check in arena allocation --- src/arena.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/arena.c b/src/arena.c index acb92243..55747bb1 100644 --- a/src/arena.c +++ b/src/arena.c @@ -107,6 +107,7 @@ static bool mi_arena_alloc(mi_arena_t* arena, size_t blocks, mi_bitmap_index_t* size_t idx = mi_atomic_read(&arena->search_idx); // start from last search for (size_t visited = 0; visited < fcount; visited++, idx++) { if (idx >= fcount) idx = 0; // wrap around + // try to atomically claim a range of bits if (mi_bitmap_try_find_claim_field(arena->blocks_inuse, idx, blocks, bitmap_idx)) { mi_atomic_write(&arena->search_idx, idx); // start search from here next time return true; @@ -135,8 +136,8 @@ static void* mi_arena_alloc_from(mi_arena_t* arena, size_t arena_index, size_t n // always committed *commit = true; } - else if (commit) { - // ensure commit now + else if (*commit) { + // arena not committed as a whole, but commit requested: ensure commit now bool any_uncommitted; mi_bitmap_claim(arena->blocks_committed, arena->field_count, needed_bcount, bitmap_index, &any_uncommitted); if (any_uncommitted) { From 2b667bd3aef92ebda22a660e068798ce31b6eed4 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Sat, 25 Jan 2020 14:47:09 +0000 Subject: [PATCH 19/28] enable arc4random abi under apple --- src/random.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/random.c b/src/random.c index c40a96da..6fef2434 100644 --- a/src/random.c +++ b/src/random.c @@ -176,7 +176,7 @@ static bool os_random_buf(void* buf, size_t buf_len) { return true; } */ -#elif defined(ANDROID) || defined(XP_DARWIN) || defined(__DragonFly__) || \ +#elif defined(ANDROID) || defined(XP_DARWIN) || defined(__APPLE__) || defined(__DragonFly__) || \ defined(__FreeBSD__) || defined(__NetBSD__) || defined(__OpenBSD__) || \ defined(__wasi__) #include @@ -325,4 +325,4 @@ static void chacha_test(void) chacha_block(&r); mi_assert_internal(array_equals(r.output, r_out, 16)); } -*/ \ No newline at end of file +*/ From 5e32d00aab55449acfd2658256a7d6ddb1d1f446 Mon Sep 17 00:00:00 2001 From: daan Date: Sat, 25 Jan 2020 12:26:08 -0800 Subject: [PATCH 20/28] add visit count to abandoned to limit list length --- include/mimalloc-types.h | 6 +++-- src/segment.c | 57 +++++++++++++++++++++++++++------------- test/test-stress.c | 7 ++--- 3 files changed, 47 insertions(+), 23 deletions(-) diff --git a/include/mimalloc-types.h b/include/mimalloc-types.h index 0c6dc666..48d86a25 100644 --- a/include/mimalloc-types.h +++ b/include/mimalloc-types.h @@ -241,10 +241,12 @@ typedef struct mi_segment_s { bool mem_is_committed; // `true` if the whole segment is eagerly committed // segment fields - struct mi_segment_s* next; // must be the first segment field -- see `segment.c:segment_alloc` + struct mi_segment_s* next; // must be the first segment field -- see `segment.c:segment_alloc` struct mi_segment_s* prev; struct mi_segment_s* abandoned_next; - size_t abandoned; // abandoned pages (i.e. the original owning thread stopped) (`abandoned <= used`) + size_t abandoned; // abandoned pages (i.e. the original owning thread stopped) (`abandoned <= used`) + size_t abandoned_visits; // count how often this segment is visited in the abandoned list (to force reclaim it it is too long) + size_t used; // count of pages in use (`used <= capacity`) size_t capacity; // count of available pages (`#free + used`) size_t segment_size;// for huge pages this may be different from `MI_SEGMENT_SIZE` diff --git a/src/segment.c b/src/segment.c index f6554520..715d632a 100644 --- a/src/segment.c +++ b/src/segment.c @@ -831,6 +831,14 @@ We use tagged pointers to avoid accidentially identifying reused segments, much like stamped references in Java. Secondly, we maintain a reader counter to avoid resetting or decommitting segments that have a pending read operation. + +Note: the current implementation is one possible design; +another way might be to keep track of abandoned segments +in the regions. This would have the advantage of keeping +all concurrent code in one place and not needing to deal +with ABA issues. The drawback is that it is unclear how to +scan abandoned segments efficiently in that case as they +would be spread among all other segments in the regions. ----------------------------------------------------------- */ // Use the bottom 20-bits (on 64-bit) of the aligned segment pointers @@ -986,6 +994,7 @@ static void mi_segment_abandon(mi_segment_t* segment, mi_segments_tld_t* tld) { mi_segments_track_size(-((long)segment->segment_size), tld); segment->thread_id = 0; segment->abandoned_next = NULL; + segment->abandoned_visits = 0; mi_abandoned_push(segment); } @@ -1009,6 +1018,7 @@ void _mi_segment_page_abandon(mi_page_t* page, mi_segments_tld_t* tld) { Reclaim abandoned pages ----------------------------------------------------------- */ +// Possibly clear pages and check if free space is available static bool mi_segment_pages_collect(mi_segment_t* segment, size_t block_size, mi_segments_tld_t* tld) { mi_assert_internal(block_size < MI_HUGE_BLOCK_SIZE); @@ -1045,13 +1055,13 @@ static bool mi_segment_pages_collect(mi_segment_t* segment, size_t block_size, m #define MI_RECLAIMED ((mi_segment_t*)1) -static mi_segment_t* mi_segment_reclaim(mi_segment_t* segment, mi_heap_t* heap, size_t block_size, mi_page_kind_t page_kind, mi_segments_tld_t* tld) { - UNUSED_RELEASE(page_kind); - mi_assert_internal(page_kind == segment->page_kind); +// Reclaim a segment +static mi_segment_t* mi_segment_reclaim(mi_segment_t* segment, mi_heap_t* heap, size_t block_size, mi_segments_tld_t* tld) { mi_assert_internal(segment->abandoned_next == NULL); bool right_page_reclaimed = false; segment->thread_id = _mi_thread_id(); + segment->abandoned_visits = 0; mi_segments_track_size((long)segment->segment_size, tld); mi_assert_internal(segment->next == NULL && segment->prev == NULL); mi_assert_expensive(mi_segment_is_valid(segment, tld)); @@ -1104,20 +1114,45 @@ static mi_segment_t* mi_segment_reclaim(mi_segment_t* segment, mi_heap_t* heap, } } +// Reclaim a segment without returning it +static void mi_segment_reclaim_force(mi_segment_t* segment, mi_heap_t* heap, mi_segments_tld_t* tld) { + mi_segment_t* res = mi_segment_reclaim(segment, heap, 0, tld); + mi_assert_internal(res != MI_RECLAIMED); // due to block_size == 0 + if (res!=MI_RECLAIMED && res != NULL) { + mi_assert_internal(res == segment); + if (res->page_kind <= MI_PAGE_MEDIUM && mi_segment_has_free(res)) { + mi_segment_insert_in_free_queue(res, tld); + } + } +} + +void _mi_abandoned_reclaim_all(mi_heap_t* heap, mi_segments_tld_t* tld) { + mi_segment_t* segment; + while ((segment = mi_abandoned_pop()) != NULL) { + mi_segment_reclaim_force(segment, heap, tld); + } +} + + static mi_segment_t* mi_segment_try_reclaim(mi_heap_t* heap, size_t block_size, mi_page_kind_t page_kind, mi_segments_tld_t* tld) { mi_segment_t* segment; int max_tries = 8; // limit the work to bound allocation times while ((max_tries-- > 0) && ((segment = mi_abandoned_pop()) != NULL)) { + segment->abandoned_visits++; bool has_page = mi_segment_pages_collect(segment,block_size,tld); // try to free up pages (due to concurrent frees) if (has_page && segment->page_kind == page_kind) { // found a free page of the right kind, or page of the right block_size with free space - return mi_segment_reclaim(segment, heap, block_size, page_kind, tld); + return mi_segment_reclaim(segment, heap, block_size, tld); } else if (segment->used==0) { // free the segment to make it available to other threads mi_segment_os_free(segment, segment->segment_size, tld); } + else if (segment->abandoned_visits >= 3) { + // always reclaim on 3rd visit to limit the list length + mi_segment_reclaim_force(segment, heap, tld); + } else { // push on the visited list so it gets not looked at too quickly again mi_abandoned_visited_push(segment); @@ -1126,20 +1161,6 @@ static mi_segment_t* mi_segment_try_reclaim(mi_heap_t* heap, size_t block_size, return NULL; } -void _mi_abandoned_reclaim_all(mi_heap_t* heap, mi_segments_tld_t* tld) { - mi_segment_t* segment; - while ((segment = mi_abandoned_pop()) != NULL) { - mi_segment_t* res = mi_segment_reclaim(segment, heap, 0, segment->page_kind, tld); - mi_assert_internal(res != NULL); - if (res != MI_RECLAIMED && res != NULL) { - mi_assert_internal(res == segment); - if (segment->page_kind <= MI_PAGE_MEDIUM && mi_segment_has_free(segment)) { - mi_segment_insert_in_free_queue(segment, tld); - } - } - } -} - /* ----------------------------------------------------------- Reclaim or allocate diff --git a/test/test-stress.c b/test/test-stress.c index 19f10360..ab4571db 100644 --- a/test/test-stress.c +++ b/test/test-stress.c @@ -32,7 +32,7 @@ static int ITER = 50; // N full iterations destructing and re-creating a // static int THREADS = 8; // more repeatable if THREADS <= #processors // static int SCALE = 100; // scaling factor -// #define STRESS // undefine for leak test +#define STRESS // undefine for leak test static bool allow_large_objects = true; // allow very large objects? static size_t use_one_size = 0; // use single object size of `N * sizeof(uintptr_t)`? @@ -124,7 +124,7 @@ static void free_items(void* p) { static void stress(intptr_t tid) { //bench_start_thread(); - uintptr_t r = (tid * 43); // ^ ticks(); + uintptr_t r = (tid * 43); // rand(); const size_t max_item_shift = 5; // 128 const size_t max_item_retained_shift = max_item_shift + 2; size_t allocs = 100 * ((size_t)SCALE) * (tid % 8 + 1); // some threads do more @@ -180,7 +180,8 @@ static void stress(intptr_t tid) { static void run_os_threads(size_t nthreads, void (*entry)(intptr_t tid)); static void test_stress(void) { - uintptr_t r = 43 * 43; + srand(0x7feb352d); + uintptr_t r = rand(); for (int n = 0; n < ITER; n++) { run_os_threads(THREADS, &stress); for (int i = 0; i < TRANSFERS; i++) { From f4630d43a71409f1963b910ffb247e137c42d85c Mon Sep 17 00:00:00 2001 From: daan Date: Sat, 25 Jan 2020 12:49:14 -0800 Subject: [PATCH 21/28] allow reset on large pages; check commit status before reset --- src/segment.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/segment.c b/src/segment.c index 715d632a..2d2263ea 100644 --- a/src/segment.c +++ b/src/segment.c @@ -231,7 +231,7 @@ static void mi_segment_protect(mi_segment_t* segment, bool protect, mi_os_tld_t* static void mi_page_reset(mi_segment_t* segment, mi_page_t* page, size_t size, mi_segments_tld_t* tld) { mi_assert_internal(page->is_committed); if (!mi_option_is_enabled(mi_option_page_reset)) return; - if (segment->mem_is_fixed || page->segment_in_use || page->is_reset) return; + if (segment->mem_is_fixed || page->segment_in_use || !page->is_committed || page->is_reset) return; size_t psize; void* start = mi_segment_raw_page_start(segment, page, &psize); page->is_reset = true; @@ -281,12 +281,12 @@ static bool mi_page_reset_is_expired(mi_page_t* page, mi_msecs_t now) { } static void mi_pages_reset_add(mi_segment_t* segment, mi_page_t* page, mi_segments_tld_t* tld) { - mi_assert_internal(!page->segment_in_use); + mi_assert_internal(!page->segment_in_use || !page->is_committed); mi_assert_internal(mi_page_not_in_queue(page,tld)); mi_assert_expensive(!mi_pages_reset_contains(page, tld)); mi_assert_internal(_mi_page_segment(page)==segment); if (!mi_option_is_enabled(mi_option_page_reset)) return; - if (segment->mem_is_fixed || page->segment_in_use || page->is_reset) return; + if (segment->mem_is_fixed || page->segment_in_use || !page->is_committed || page->is_reset) return; if (mi_option_get(mi_option_reset_delay) == 0) { // reset immediately? @@ -782,7 +782,7 @@ static void mi_segment_page_clear(mi_segment_t* segment, mi_page_t* page, bool a segment->used--; // add to the free page list for reuse/reset - if (allow_reset && segment->page_kind <= MI_PAGE_MEDIUM) { + if (allow_reset) { mi_pages_reset_add(segment, page, tld); } } @@ -1095,7 +1095,10 @@ static mi_segment_t* mi_segment_reclaim(mi_segment_t* segment, mi_heap_t* heap, right_page_reclaimed = true; } } - } + } + else if (page->is_committed && !page->is_reset) { // not in-use, and not reset yet + mi_pages_reset_add(segment, page, tld); + } } mi_assert_internal(segment->abandoned == 0); if (right_page_reclaimed) { From 19a0d9dfa0f1ed1145d6943d971511b2a2d1060d Mon Sep 17 00:00:00 2001 From: daan Date: Sat, 25 Jan 2020 12:51:56 -0800 Subject: [PATCH 22/28] clean up stress test --- test/test-stress.c | 29 +++++------------------------ 1 file changed, 5 insertions(+), 24 deletions(-) diff --git a/test/test-stress.c b/test/test-stress.c index ab4571db..1b559a59 100644 --- a/test/test-stress.c +++ b/test/test-stress.c @@ -59,7 +59,6 @@ const uintptr_t cookie = 0xbf58476d1ce4e5b9UL; const uintptr_t cookie = 0x1ce4e5b9UL; #endif -static uintptr_t ticks(void); static void* atomic_exchange_ptr(volatile void** p, void* newval); typedef uintptr_t* random_t; @@ -180,7 +179,6 @@ static void stress(intptr_t tid) { static void run_os_threads(size_t nthreads, void (*entry)(intptr_t tid)); static void test_stress(void) { - srand(0x7feb352d); uintptr_t r = rand(); for (int n = 0; n < ITER; n++) { run_os_threads(THREADS, &stress); @@ -197,8 +195,9 @@ static void test_stress(void) { } } +#ifndef STRESS static void leak(intptr_t tid) { - uintptr_t r = (43*tid)^ticks(); + uintptr_t r = rand(); void* p = alloc_items(1 /*pick(&r)%128*/, &r); if (chance(50, &r)) { intptr_t i = (pick(&r) % TRANSFERS); @@ -207,7 +206,7 @@ static void leak(intptr_t tid) { } } -static void test_leak(void) { +static void test_leak(void) { for (int n = 0; n < ITER; n++) { run_os_threads(THREADS, &leak); mi_collect(false); @@ -216,6 +215,7 @@ static void test_leak(void) { #endif } } +#endif int main(int argc, char** argv) { // > mimalloc-test-stress [THREADS] [SCALE] [ITER] @@ -241,6 +241,7 @@ int main(int argc, char** argv) { //bench_start_program(); // Run ITER full iterations where half the objects in the transfer buffer survive to the next round. + srand(0x7feb352d); mi_stats_reset(); #ifdef STRESS test_stress(); @@ -261,12 +262,6 @@ static void (*thread_entry_fun)(intptr_t) = &stress; #include -static uintptr_t ticks(void) { - LARGE_INTEGER t; - QueryPerformanceCounter(&t); - return (uintptr_t)t.QuadPart; -} - static DWORD WINAPI thread_entry(LPVOID param) { thread_entry_fun((intptr_t)param); return 0; @@ -331,18 +326,4 @@ static void* atomic_exchange_ptr(volatile void** p, void* newval) { } #endif -#include -#ifdef CLOCK_REALTIME -uintptr_t ticks(void) { - struct timespec t; - clock_gettime(CLOCK_REALTIME, &t); - return ((uintptr_t)t.tv_sec * 1000) + ((uintptr_t)t.tv_nsec / 1000000); -} -#else -// low resolution timer -uintptr_t _mi_clock_now(void) { - return ((uintptr_t)clock() / ((uintptr_t)CLOCKS_PER_SEC / 1000)); -} -#endif - #endif From 62b8fb26b11f7b5e496add0cc6c9c1c9da3e0791 Mon Sep 17 00:00:00 2001 From: daan Date: Sat, 25 Jan 2020 13:27:47 -0800 Subject: [PATCH 23/28] fix freeing of segments on forced reclaim --- src/{memory.c => region.c} | 0 src/segment.c | 5 ++++- 2 files changed, 4 insertions(+), 1 deletion(-) rename src/{memory.c => region.c} (100%) diff --git a/src/memory.c b/src/region.c similarity index 100% rename from src/memory.c rename to src/region.c diff --git a/src/segment.c b/src/segment.c index 2d2263ea..e536ae59 100644 --- a/src/segment.c +++ b/src/segment.c @@ -1123,7 +1123,10 @@ static void mi_segment_reclaim_force(mi_segment_t* segment, mi_heap_t* heap, mi_ mi_assert_internal(res != MI_RECLAIMED); // due to block_size == 0 if (res!=MI_RECLAIMED && res != NULL) { mi_assert_internal(res == segment); - if (res->page_kind <= MI_PAGE_MEDIUM && mi_segment_has_free(res)) { + if (res->used == 0) { + mi_segment_free(segment, false, tld); + } + else if (res->page_kind <= MI_PAGE_MEDIUM && mi_segment_has_free(res)) { mi_segment_insert_in_free_queue(res, tld); } } From 7785139201dbac8bc9515d7f5fa148f3e0c7827d Mon Sep 17 00:00:00 2001 From: daan Date: Sat, 25 Jan 2020 13:28:24 -0800 Subject: [PATCH 24/28] fix warning on gcc on attribute ignore in templates --- src/arena.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/arena.c b/src/arena.c index 21d0affc..7bf8099b 100644 --- a/src/arena.c +++ b/src/arena.c @@ -54,7 +54,7 @@ bool _mi_os_commit(void* p, size_t size, bool* is_zero, mi_stats_t* stats); #define MI_MAX_ARENAS (64) // not more than 256 (since we use 8 bits in the memid) // A memory arena descriptor -typedef mi_decl_cache_align struct mi_arena_s { +typedef struct mi_arena_s { _Atomic(uint8_t*) start; // the start of the memory area size_t block_count; // size of the area in arena blocks (of `MI_ARENA_BLOCK_SIZE`) size_t field_count; // number of bitmap fields (where `field_count * MI_BITMAP_FIELD_BITS >= block_count`) From 4faf412f53ac49ee04584b015c826a7bb1d67177 Mon Sep 17 00:00:00 2001 From: daan Date: Sat, 25 Jan 2020 13:28:49 -0800 Subject: [PATCH 25/28] move 'memory.c' to 'region.c' --- CMakeLists.txt | 2 +- ide/vs2017/mimalloc-override.vcxproj | 4 ++-- ide/vs2017/mimalloc-override.vcxproj.filters | 4 ++-- ide/vs2017/mimalloc.vcxproj | 4 ++-- ide/vs2017/mimalloc.vcxproj.filters | 4 ++-- ide/vs2019/mimalloc-override.vcxproj | 4 ++-- ide/vs2019/mimalloc-override.vcxproj.filters | 4 ++-- ide/vs2019/mimalloc.vcxproj | 4 ++-- ide/vs2019/mimalloc.vcxproj.filters | 4 ++-- src/static.c | 2 +- 10 files changed, 18 insertions(+), 18 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 95318a0e..b60e64a4 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -21,7 +21,7 @@ set(mi_sources src/random.c src/os.c src/arena.c - src/memory.c + src/region.c src/segment.c src/page.c src/alloc.c diff --git a/ide/vs2017/mimalloc-override.vcxproj b/ide/vs2017/mimalloc-override.vcxproj index 4225a2f9..26c8080b 100644 --- a/ide/vs2017/mimalloc-override.vcxproj +++ b/ide/vs2017/mimalloc-override.vcxproj @@ -234,7 +234,7 @@ - + @@ -251,4 +251,4 @@ - \ No newline at end of file + diff --git a/ide/vs2017/mimalloc-override.vcxproj.filters b/ide/vs2017/mimalloc-override.vcxproj.filters index 037fbcbb..02652658 100644 --- a/ide/vs2017/mimalloc-override.vcxproj.filters +++ b/ide/vs2017/mimalloc-override.vcxproj.filters @@ -61,7 +61,7 @@ Source Files - + Source Files @@ -77,4 +77,4 @@ Source Files - \ No newline at end of file + diff --git a/ide/vs2017/mimalloc.vcxproj b/ide/vs2017/mimalloc.vcxproj index e08deec4..9d6af0e5 100644 --- a/ide/vs2017/mimalloc.vcxproj +++ b/ide/vs2017/mimalloc.vcxproj @@ -220,7 +220,7 @@ - + true @@ -245,4 +245,4 @@ - \ No newline at end of file + diff --git a/ide/vs2017/mimalloc.vcxproj.filters b/ide/vs2017/mimalloc.vcxproj.filters index 5fe74aa0..43660519 100644 --- a/ide/vs2017/mimalloc.vcxproj.filters +++ b/ide/vs2017/mimalloc.vcxproj.filters @@ -47,7 +47,7 @@ Source Files - + Source Files @@ -80,4 +80,4 @@ Header Files - \ No newline at end of file + diff --git a/ide/vs2019/mimalloc-override.vcxproj b/ide/vs2019/mimalloc-override.vcxproj index ac19e321..17b6f4c0 100644 --- a/ide/vs2019/mimalloc-override.vcxproj +++ b/ide/vs2019/mimalloc-override.vcxproj @@ -237,7 +237,7 @@ - + @@ -254,4 +254,4 @@ - \ No newline at end of file + diff --git a/ide/vs2019/mimalloc-override.vcxproj.filters b/ide/vs2019/mimalloc-override.vcxproj.filters index a8c5a5de..83d6f7fe 100644 --- a/ide/vs2019/mimalloc-override.vcxproj.filters +++ b/ide/vs2019/mimalloc-override.vcxproj.filters @@ -22,7 +22,7 @@ Source Files - + Source Files @@ -78,4 +78,4 @@ {39cb7e38-69d0-43fb-8406-6a0f7cefc3b4} - \ No newline at end of file + diff --git a/ide/vs2019/mimalloc.vcxproj b/ide/vs2019/mimalloc.vcxproj index a98e78ba..a1372204 100644 --- a/ide/vs2019/mimalloc.vcxproj +++ b/ide/vs2019/mimalloc.vcxproj @@ -223,7 +223,7 @@ - + true @@ -248,4 +248,4 @@ - \ No newline at end of file + diff --git a/ide/vs2019/mimalloc.vcxproj.filters b/ide/vs2019/mimalloc.vcxproj.filters index 61de4afe..4704fb2e 100644 --- a/ide/vs2019/mimalloc.vcxproj.filters +++ b/ide/vs2019/mimalloc.vcxproj.filters @@ -22,7 +22,7 @@ Source Files - + Source Files @@ -81,4 +81,4 @@ {852a14ae-6dde-4e95-8077-ca705e97e5af} - \ No newline at end of file + diff --git a/src/static.c b/src/static.c index 0519453e..ec9370eb 100644 --- a/src/static.c +++ b/src/static.c @@ -17,7 +17,7 @@ terms of the MIT license. A copy of the license can be found in the file #include "random.c" #include "os.c" #include "arena.c" -#include "memory.c" +#include "region.c" #include "segment.c" #include "page.c" #include "heap.c" From 394b796ea0aec69b2f97ad51cce16ed432ca6e69 Mon Sep 17 00:00:00 2001 From: daan Date: Sat, 25 Jan 2020 13:43:56 -0800 Subject: [PATCH 26/28] fix over-eager page reset in segment reclamation --- src/segment.c | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/src/segment.c b/src/segment.c index e536ae59..194aa793 100644 --- a/src/segment.c +++ b/src/segment.c @@ -1019,26 +1019,18 @@ void _mi_segment_page_abandon(mi_page_t* page, mi_segments_tld_t* tld) { ----------------------------------------------------------- */ // Possibly clear pages and check if free space is available -static bool mi_segment_pages_collect(mi_segment_t* segment, size_t block_size, mi_segments_tld_t* tld) +static bool mi_segment_check_free(mi_segment_t* segment, size_t block_size) { mi_assert_internal(block_size < MI_HUGE_BLOCK_SIZE); bool has_page = false; for (size_t i = 0; i < segment->capacity; i++) { mi_page_t* page = &segment->pages[i]; - if (page->segment_in_use) { - mi_assert_internal(!page->is_reset); - mi_assert_internal(page->is_committed); - mi_assert_internal(mi_page_not_in_queue(page, tld)); - mi_assert_internal(mi_page_thread_free_flag(page)==MI_NEVER_DELAYED_FREE); - mi_assert_internal(mi_page_heap(page) == NULL); - mi_assert_internal(page->next == NULL); + if (page->segment_in_use) { // ensure used count is up to date and collect potential concurrent frees _mi_page_free_collect(page, false); if (mi_page_all_free(page)) { - // if everything free already, clear the page directly - segment->abandoned--; - _mi_stat_decrease(&tld->stats->pages_abandoned, 1); - mi_segment_page_clear(segment, page, false, tld); // no (delayed) reset allowed (as the segment is still abandoned) + // if everything free already, page can be reused for some block size + // note: don't clear yet as we can only reset it once it is reclaimed has_page = true; } else if (page->xblock_size == block_size && page->used < page->reserved) { @@ -1047,6 +1039,7 @@ static bool mi_segment_pages_collect(mi_segment_t* segment, size_t block_size, m } } else { + // whole empty page has_page = true; } } @@ -1081,7 +1074,6 @@ static mi_segment_t* mi_segment_reclaim(mi_segment_t* segment, mi_heap_t* heap, // set the heap again and allow delayed free again mi_page_set_heap(page, heap); _mi_page_use_delayed_free(page, MI_USE_DELAYED_FREE, true); // override never (after heap is set) - mi_assert_internal(!mi_page_all_free(page)); // TODO: should we not collect again given that we just collected? _mi_page_free_collect(page, false); // ensure used count is up to date if (mi_page_all_free(page)) { @@ -1097,7 +1089,8 @@ static mi_segment_t* mi_segment_reclaim(mi_segment_t* segment, mi_heap_t* heap, } } else if (page->is_committed && !page->is_reset) { // not in-use, and not reset yet - mi_pages_reset_add(segment, page, tld); + // note: no not reset as this includes pages that were not touched before + // mi_pages_reset_add(segment, page, tld); } } mi_assert_internal(segment->abandoned == 0); @@ -1146,7 +1139,7 @@ static mi_segment_t* mi_segment_try_reclaim(mi_heap_t* heap, size_t block_size, int max_tries = 8; // limit the work to bound allocation times while ((max_tries-- > 0) && ((segment = mi_abandoned_pop()) != NULL)) { segment->abandoned_visits++; - bool has_page = mi_segment_pages_collect(segment,block_size,tld); // try to free up pages (due to concurrent frees) + bool has_page = mi_segment_check_free(segment,block_size); // try to free up pages (due to concurrent frees) if (has_page && segment->page_kind == page_kind) { // found a free page of the right kind, or page of the right block_size with free space return mi_segment_reclaim(segment, heap, block_size, tld); From d4927adddc2c3b748934d3e45c4ddb673c6076ee Mon Sep 17 00:00:00 2001 From: daan Date: Sat, 25 Jan 2020 14:30:02 -0800 Subject: [PATCH 27/28] add extra assertion that all segments are free on thread termination --- src/init.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/init.c b/src/init.c index 366acbf5..f8411187 100644 --- a/src/init.c +++ b/src/init.c @@ -203,6 +203,7 @@ static bool _mi_heap_done(mi_heap_t* heap) { // free if not the main thread if (heap != &_mi_heap_main) { + mi_assert_internal(heap->tld->segments.count == 0); _mi_os_free(heap, sizeof(mi_thread_data_t), &_mi_stats_main); } #if (MI_DEBUG > 0) From e628fc70676e8e2176fe66e8275480c14ad29ca3 Mon Sep 17 00:00:00 2001 From: daan Date: Sun, 26 Jan 2020 12:39:11 -0800 Subject: [PATCH 28/28] cleanup reclaim logic --- include/mimalloc-internal.h | 24 +++----- src/page.c | 40 +++++------- src/segment.c | 117 +++++++++++++++++++----------------- 3 files changed, 87 insertions(+), 94 deletions(-) diff --git a/include/mimalloc-internal.h b/include/mimalloc-internal.h index 902d2fdf..c7d7a1da 100644 --- a/include/mimalloc-internal.h +++ b/include/mimalloc-internal.h @@ -411,30 +411,24 @@ static inline mi_thread_free_t mi_tf_set_block(mi_thread_free_t tf, mi_block_t* return mi_tf_make(block, mi_tf_delayed(tf)); } -// are all blocks in a page freed? +// are all blocks in a page freed? +// note: needs up-to-date used count, (as the `xthread_free` list may not be empty). see `_mi_page_collect_free`. static inline bool mi_page_all_free(const mi_page_t* page) { mi_assert_internal(page != NULL); return (page->used == 0); } -// are there immediately available blocks +// are there any available blocks? +static inline bool mi_page_has_any_available(const mi_page_t* page) { + mi_assert_internal(page != NULL && page->reserved > 0); + return (page->used < page->reserved || (mi_page_thread_free(page) != NULL)); +} + +// are there immediately available blocks, i.e. blocks available on the free list. static inline bool mi_page_immediate_available(const mi_page_t* page) { mi_assert_internal(page != NULL); return (page->free != NULL); } -// are there free blocks in this page? -static inline bool mi_page_has_free(mi_page_t* page) { - mi_assert_internal(page != NULL); - bool hasfree = (mi_page_immediate_available(page) || page->local_free != NULL || (mi_page_thread_free(page) != NULL)); - mi_assert_internal(hasfree || page->used == page->capacity); - return hasfree; -} - -// are all blocks in use? -static inline bool mi_page_all_used(mi_page_t* page) { - mi_assert_internal(page != NULL); - return !mi_page_has_free(page); -} // is more than 7/8th of a page in use? static inline bool mi_page_mostly_used(const mi_page_t* page) { diff --git a/src/page.c b/src/page.c index c5b86b08..e552a61e 100644 --- a/src/page.c +++ b/src/page.c @@ -234,6 +234,7 @@ void _mi_page_reclaim(mi_heap_t* heap, mi_page_t* page) { mi_assert_internal(mi_page_thread_free_flag(page) != MI_NEVER_DELAYED_FREE); mi_assert_internal(_mi_page_segment(page)->page_kind != MI_PAGE_HUGE); mi_assert_internal(!page->is_reset); + // TODO: push on full queue immediately if it is full? mi_page_queue_t* pq = mi_page_queue(heap, mi_page_block_size(page)); mi_page_queue_push(heap, pq, page); mi_assert_expensive(_mi_page_is_valid(page)); @@ -245,28 +246,16 @@ static mi_page_t* mi_page_fresh_alloc(mi_heap_t* heap, mi_page_queue_t* pq, size mi_assert_internal(pq==NULL||block_size == pq->block_size); mi_page_t* page = _mi_segment_page_alloc(heap, block_size, &heap->tld->segments, &heap->tld->os); if (page == NULL) { - // this may be out-of-memory, or a page was reclaimed - if (pq!=NULL && (page = pq->first) != NULL) { - mi_assert_expensive(_mi_page_is_valid(page)); - if (!mi_page_immediate_available(page)) { - mi_page_extend_free(heap, page, heap->tld); - } - mi_assert_internal(mi_page_immediate_available(page)); - if (mi_page_immediate_available(page)) { - return page; // reclaimed page - } - } - return NULL; // out-of-memory - } - else { - // a fresh page was allocated, initialize it - mi_assert_internal(pq==NULL || _mi_page_segment(page)->page_kind != MI_PAGE_HUGE); - mi_page_init(heap, page, block_size, heap->tld); - _mi_stat_increase(&heap->tld->stats.pages, 1); - if (pq!=NULL) mi_page_queue_push(heap, pq, page); // huge pages use pq==NULL - mi_assert_expensive(_mi_page_is_valid(page)); - return page; + // this may be out-of-memory, or an abandoned page was reclaimed (and in our queue) + return NULL; } + // a fresh page was found, initialize it + mi_assert_internal(pq==NULL || _mi_page_segment(page)->page_kind != MI_PAGE_HUGE); + mi_page_init(heap, page, block_size, heap->tld); + _mi_stat_increase(&heap->tld->stats.pages, 1); + if (pq!=NULL) mi_page_queue_push(heap, pq, page); // huge pages use pq==NULL + mi_assert_expensive(_mi_page_is_valid(page)); + return page; } // Get a fresh page to use @@ -648,7 +637,7 @@ static void mi_page_init(mi_heap_t* heap, mi_page_t* page, size_t block_size, mi -------------------------------------------------------------*/ // Find a page with free blocks of `page->block_size`. -static mi_page_t* mi_page_queue_find_free_ex(mi_heap_t* heap, mi_page_queue_t* pq) +static mi_page_t* mi_page_queue_find_free_ex(mi_heap_t* heap, mi_page_queue_t* pq, bool first_try) { // search through the pages in "next fit" order size_t count = 0; @@ -686,13 +675,16 @@ static mi_page_t* mi_page_queue_find_free_ex(mi_heap_t* heap, mi_page_queue_t* p if (page == NULL) { _mi_heap_collect_retired(heap, false); // perhaps make a page available page = mi_page_fresh(heap, pq); + if (page == NULL && first_try) { + // out-of-memory _or_ an abandoned page with free blocks was reclaimed, try once again + page = mi_page_queue_find_free_ex(heap, pq, false); + } } else { mi_assert(pq->first == page); page->retire_expire = 0; } mi_assert_internal(page == NULL || mi_page_immediate_available(page)); - return page; } @@ -716,7 +708,7 @@ static inline mi_page_t* mi_find_free_page(mi_heap_t* heap, size_t size) { return page; // fast path } } - return mi_page_queue_find_free_ex(heap, pq); + return mi_page_queue_find_free_ex(heap, pq, true); } diff --git a/src/segment.c b/src/segment.c index 194aa793..c7a9662b 100644 --- a/src/segment.c +++ b/src/segment.c @@ -669,6 +669,11 @@ static mi_segment_t* mi_segment_init(mi_segment_t* segment, size_t required, mi_ // set protection mi_segment_protect(segment, true, tld->os); + // insert in free lists for small and medium pages + if (page_kind <= MI_PAGE_MEDIUM) { + mi_segment_insert_in_free_queue(segment, tld); + } + //fprintf(stderr,"mimalloc: alloc segment at %p\n", (void*)segment); return segment; } @@ -1019,21 +1024,25 @@ void _mi_segment_page_abandon(mi_page_t* page, mi_segments_tld_t* tld) { ----------------------------------------------------------- */ // Possibly clear pages and check if free space is available -static bool mi_segment_check_free(mi_segment_t* segment, size_t block_size) +static bool mi_segment_check_free(mi_segment_t* segment, size_t block_size, bool* all_pages_free) { mi_assert_internal(block_size < MI_HUGE_BLOCK_SIZE); bool has_page = false; + size_t pages_used = 0; + size_t pages_used_empty = 0; for (size_t i = 0; i < segment->capacity; i++) { mi_page_t* page = &segment->pages[i]; if (page->segment_in_use) { + pages_used++; // ensure used count is up to date and collect potential concurrent frees _mi_page_free_collect(page, false); if (mi_page_all_free(page)) { // if everything free already, page can be reused for some block size - // note: don't clear yet as we can only reset it once it is reclaimed + // note: don't clear the page yet as we can only OS reset it once it is reclaimed + pages_used_empty++; has_page = true; } - else if (page->xblock_size == block_size && page->used < page->reserved) { + else if (page->xblock_size == block_size && mi_page_has_any_available(page)) { // a page has available free blocks of the right size has_page = true; } @@ -1043,15 +1052,19 @@ static bool mi_segment_check_free(mi_segment_t* segment, size_t block_size) has_page = true; } } + mi_assert_internal(pages_used == segment->used && pages_used >= pages_used_empty); + if (all_pages_free != NULL) { + *all_pages_free = ((pages_used - pages_used_empty) == 0); + } return has_page; } -#define MI_RECLAIMED ((mi_segment_t*)1) -// Reclaim a segment -static mi_segment_t* mi_segment_reclaim(mi_segment_t* segment, mi_heap_t* heap, size_t block_size, mi_segments_tld_t* tld) { +// Reclaim a segment; returns NULL if the segment was freed +// set `right_page_reclaimed` to `true` if it reclaimed a page of the right `block_size` that was not full. +static mi_segment_t* mi_segment_reclaim(mi_segment_t* segment, mi_heap_t* heap, size_t requested_block_size, bool* right_page_reclaimed, mi_segments_tld_t* tld) { mi_assert_internal(segment->abandoned_next == NULL); - bool right_page_reclaimed = false; + if (right_page_reclaimed != NULL) { *right_page_reclaimed = false; } segment->thread_id = _mi_thread_id(); segment->abandoned_visits = 0; @@ -1071,10 +1084,10 @@ static mi_segment_t* mi_segment_reclaim(mi_segment_t* segment, mi_heap_t* heap, segment->abandoned--; mi_assert(page->next == NULL); _mi_stat_decrease(&tld->stats->pages_abandoned, 1); - // set the heap again and allow delayed free again + // set the heap again and allow heap thread delayed free again. mi_page_set_heap(page, heap); _mi_page_use_delayed_free(page, MI_USE_DELAYED_FREE, true); // override never (after heap is set) - // TODO: should we not collect again given that we just collected? + // TODO: should we not collect again given that we just collected in `check_free`? _mi_page_free_collect(page, false); // ensure used count is up to date if (mi_page_all_free(page)) { // if everything free already, clear the page directly @@ -1083,77 +1096,67 @@ static mi_segment_t* mi_segment_reclaim(mi_segment_t* segment, mi_heap_t* heap, else { // otherwise reclaim it into the heap _mi_page_reclaim(heap, page); - if (block_size == page->xblock_size) { - right_page_reclaimed = true; + if (requested_block_size == page->xblock_size && mi_page_has_any_available(page)) { + if (right_page_reclaimed != NULL) { *right_page_reclaimed = true; } } } } else if (page->is_committed && !page->is_reset) { // not in-use, and not reset yet - // note: no not reset as this includes pages that were not touched before + // note: do not reset as this includes pages that were not touched before // mi_pages_reset_add(segment, page, tld); } } mi_assert_internal(segment->abandoned == 0); - if (right_page_reclaimed) { - // add the segment's free pages to the free small segment queue + if (segment->used == 0) { + mi_assert_internal(right_page_reclaimed == NULL || !(*right_page_reclaimed)); + mi_segment_free(segment, false, tld); + return NULL; + } + else { if (segment->page_kind <= MI_PAGE_MEDIUM && mi_segment_has_free(segment)) { mi_segment_insert_in_free_queue(segment, tld); } - // and return reclaimed: at the page allocation the page is already in the queue now - return MI_RECLAIMED; - } - else { - // otherwise return the segment as it will contain some free pages - // (except for abandoned_reclaim_all which uses a block_size of zero) - mi_assert_internal(segment->used < segment->capacity || block_size == 0); return segment; } } -// Reclaim a segment without returning it -static void mi_segment_reclaim_force(mi_segment_t* segment, mi_heap_t* heap, mi_segments_tld_t* tld) { - mi_segment_t* res = mi_segment_reclaim(segment, heap, 0, tld); - mi_assert_internal(res != MI_RECLAIMED); // due to block_size == 0 - if (res!=MI_RECLAIMED && res != NULL) { - mi_assert_internal(res == segment); - if (res->used == 0) { - mi_segment_free(segment, false, tld); - } - else if (res->page_kind <= MI_PAGE_MEDIUM && mi_segment_has_free(res)) { - mi_segment_insert_in_free_queue(res, tld); - } - } -} void _mi_abandoned_reclaim_all(mi_heap_t* heap, mi_segments_tld_t* tld) { mi_segment_t* segment; while ((segment = mi_abandoned_pop()) != NULL) { - mi_segment_reclaim_force(segment, heap, tld); + mi_segment_reclaim(segment, heap, 0, NULL, tld); } } - -static mi_segment_t* mi_segment_try_reclaim(mi_heap_t* heap, size_t block_size, mi_page_kind_t page_kind, mi_segments_tld_t* tld) +static mi_segment_t* mi_segment_try_reclaim(mi_heap_t* heap, size_t block_size, mi_page_kind_t page_kind, bool* reclaimed, mi_segments_tld_t* tld) { + *reclaimed = false; mi_segment_t* segment; int max_tries = 8; // limit the work to bound allocation times while ((max_tries-- > 0) && ((segment = mi_abandoned_pop()) != NULL)) { segment->abandoned_visits++; - bool has_page = mi_segment_check_free(segment,block_size); // try to free up pages (due to concurrent frees) - if (has_page && segment->page_kind == page_kind) { - // found a free page of the right kind, or page of the right block_size with free space - return mi_segment_reclaim(segment, heap, block_size, tld); + bool all_pages_free; + bool has_page = mi_segment_check_free(segment,block_size,&all_pages_free); // try to free up pages (due to concurrent frees) + if (all_pages_free) { + // free the segment (by forced reclaim) to make it available to other threads. + // note1: we prefer to free a segment as that might lead to reclaiming another + // segment that is still partially used. + // note2: we could in principle optimize this by skipping reclaim and directly + // freeing but that would violate some invariants temporarily) + mi_segment_reclaim(segment, heap, 0, NULL, tld); } - else if (segment->used==0) { - // free the segment to make it available to other threads - mi_segment_os_free(segment, segment->segment_size, tld); + else if (has_page && segment->page_kind == page_kind) { + // found a free page of the right kind, or page of the right block_size with free space + // we return the result of reclaim (which is usually `segment`) as it might free + // the segment due to concurrent frees (in which case `NULL` is returned). + return mi_segment_reclaim(segment, heap, block_size, reclaimed, tld); } else if (segment->abandoned_visits >= 3) { - // always reclaim on 3rd visit to limit the list length - mi_segment_reclaim_force(segment, heap, tld); + // always reclaim on 3rd visit to limit the list length. + mi_segment_reclaim(segment, heap, 0, NULL, tld); } else { - // push on the visited list so it gets not looked at too quickly again + // otherwise, push on the visited list so it gets not looked at too quickly again mi_abandoned_visited_push(segment); } } @@ -1176,12 +1179,16 @@ static mi_segment_t* mi_segment_reclaim_or_alloc(mi_heap_t* heap, size_t block_s return segment; } // 2. try to reclaim an abandoned segment - segment = mi_segment_try_reclaim(heap, block_size, page_kind, tld); - if (segment == MI_RECLAIMED) { - return NULL; // pretend out-of-memory as the page will be in the page queue of the heap + bool reclaimed; + segment = mi_segment_try_reclaim(heap, block_size, page_kind, &reclaimed, tld); + if (reclaimed) { + // reclaimed the right page right into the heap + mi_assert_internal(segment != NULL && segment->page_kind == page_kind && page_kind <= MI_PAGE_LARGE); + return NULL; // pretend out-of-memory as the page will be in the page queue of the heap with available blocks } else if (segment != NULL) { - return segment; // reclaimed a segment with empty pages in it + // reclaimed a segment with empty pages (of `page_kind`) in it + return segment; } // 3. otherwise allocate a fresh segment return mi_segment_alloc(0, page_kind, page_shift, tld, os_tld); @@ -1216,12 +1223,12 @@ static mi_page_t* mi_segment_page_alloc(mi_heap_t* heap, size_t block_size, mi_p // find an available segment the segment free queue mi_segment_queue_t* const free_queue = mi_segment_free_queue_of_kind(kind, tld); if (mi_segment_queue_is_empty(free_queue)) { - // possibly allocate a fresh segment - mi_segment_t* segment = mi_segment_reclaim_or_alloc(heap, block_size, kind, page_shift, tld, os_tld); + // possibly allocate or reclaim a fresh segment + mi_segment_t* const segment = mi_segment_reclaim_or_alloc(heap, block_size, kind, page_shift, tld, os_tld); if (segment == NULL) return NULL; // return NULL if out-of-memory (or reclaimed) + mi_assert_internal(free_queue->first == segment); mi_assert_internal(segment->page_kind==kind); mi_assert_internal(segment->used < segment->capacity); - mi_segment_enqueue(free_queue, segment); } mi_assert_internal(free_queue->first != NULL); mi_page_t* const page = mi_segment_page_alloc_in(free_queue->first, tld);