From e3ebebb9902c56b6899f70f046cbcc8089674569 Mon Sep 17 00:00:00 2001 From: daanx Date: Sat, 21 Dec 2024 14:39:17 -0800 Subject: [PATCH] update lock primitive; fix arena exclusive allocation --- include/mimalloc/atomic.h | 31 ++++++++++++++++++++++++++++--- src/arena-abandon.c | 33 +++++++++++---------------------- src/arena.c | 5 +++-- src/init.c | 15 +++++++-------- 4 files changed, 49 insertions(+), 35 deletions(-) diff --git a/include/mimalloc/atomic.h b/include/mimalloc/atomic.h index 0c967896..733dbf42 100644 --- a/include/mimalloc/atomic.h +++ b/include/mimalloc/atomic.h @@ -1,5 +1,5 @@ /* ---------------------------------------------------------------------------- -Copyright (c) 2018-2023 Microsoft Research, Daan Leijen +Copyright (c) 2018-2024 Microsoft Research, Daan Leijen This is free software; you can redistribute it and/or modify it under the terms of the MIT license. A copy of the license can be found in the file "LICENSE" at the root of this distribution. @@ -411,8 +411,11 @@ static inline void mi_atomic_yield(void) { #pragma warning(disable:26110) // unlock with holding lock #endif +#define mi_lock(lock) for(bool _go = (mi_lock_acquire(lock),true); _go; (mi_lock_release(lock), _go=false) ) + #if defined(_WIN32) +#if 1 #define mi_lock_t SRWLOCK // slim reader-writer lock static inline bool mi_lock_try_acquire(mi_lock_t* lock) { @@ -432,6 +435,30 @@ static inline void mi_lock_done(mi_lock_t* lock) { // nothing } +#else +#define mi_lock_t CRITICAL_SECTION + +static inline bool mi_lock_try_acquire(mi_lock_t* lock) { + return TryEnterCriticalSection(lock); + +} +static inline void mi_lock_acquire(mi_lock_t* lock) { + EnterCriticalSection(lock); + +} +static inline void mi_lock_release(mi_lock_t* lock) { + LeaveCriticalSection(lock); + +} +static inline void mi_lock_init(mi_lock_t* lock) { + InitializeCriticalSection(lock); + +} +static inline void mi_lock_done(mi_lock_t* lock) { + DeleteCriticalSection(lock); + +} +#endif #elif defined(MI_USE_PTHREADS) @@ -506,6 +533,4 @@ static inline void mi_lock_done(mi_lock_t* lock) { #endif - - #endif // __MIMALLOC_ATOMIC_H diff --git a/src/arena-abandon.c b/src/arena-abandon.c index 48e37794..460c80fc 100644 --- a/src/arena-abandon.c +++ b/src/arena-abandon.c @@ -120,11 +120,7 @@ static void mi_arena_segment_os_mark_abandoned(mi_segment_t* segment) { mi_assert(segment->memid.memkind != MI_MEM_ARENA); // not in an arena; we use a list of abandoned segments mi_subproc_t* const subproc = segment->subproc; - if (!mi_lock_acquire(&subproc->abandoned_os_lock)) { - _mi_error_message(EFAULT, "internal error: failed to acquire the abandoned (os) segment lock to mark abandonment"); - // we can continue but cannot visit/reclaim such blocks.. - } - else { + mi_lock(&subproc->abandoned_os_lock) { // push on the tail of the list (important for the visitor) mi_segment_t* prev = subproc->abandoned_os_list_tail; mi_assert_internal(prev == NULL || prev->abandoned_os_next == NULL); @@ -138,7 +134,6 @@ static void mi_arena_segment_os_mark_abandoned(mi_segment_t* segment) { mi_atomic_increment_relaxed(&subproc->abandoned_os_list_count); mi_atomic_increment_relaxed(&subproc->abandoned_count); // and release the lock - mi_lock_release(&subproc->abandoned_os_lock); } return; } @@ -251,7 +246,7 @@ static mi_segment_t* mi_arena_segment_clear_abandoned_next_field(mi_arena_field_ if mi_unlikely(field != 0) { // skip zero fields quickly // we only take the arena lock if there are actually abandoned segments present if (!has_lock && mi_option_is_enabled(mi_option_visit_abandoned)) { - has_lock = (previous->visit_all ? mi_lock_acquire(&arena->abandoned_visit_lock) : mi_lock_try_acquire(&arena->abandoned_visit_lock)); + has_lock = (previous->visit_all ? (mi_lock_acquire(&arena->abandoned_visit_lock),true) : mi_lock_try_acquire(&arena->abandoned_visit_lock)); if (!has_lock) { if (previous->visit_all) { _mi_error_message(EFAULT, "internal error: failed to visit all abandoned segments due to failure to acquire the visitor lock"); @@ -289,8 +284,8 @@ static mi_segment_t* mi_arena_segment_clear_abandoned_next_list(mi_arena_field_c // we only allow one thread per sub-process to do to visit guarded by the `abandoned_os_visit_lock`. // The lock is released when the cursor is released. if (!previous->hold_visit_lock) { - previous->hold_visit_lock = (previous->visit_all ? mi_lock_acquire(&previous->subproc->abandoned_os_visit_lock) - : mi_lock_try_acquire(&previous->subproc->abandoned_os_visit_lock)); + previous->hold_visit_lock = (previous->visit_all ? (mi_lock_acquire(&previous->subproc->abandoned_os_visit_lock),true) + : mi_lock_try_acquire(&previous->subproc->abandoned_os_visit_lock)); if (!previous->hold_visit_lock) { if (previous->visit_all) { _mi_error_message(EFAULT, "internal error: failed to visit all abandoned segments due to failure to acquire the OS visitor lock"); @@ -301,21 +296,15 @@ static mi_segment_t* mi_arena_segment_clear_abandoned_next_list(mi_arena_field_c // One list entry at a time while (previous->os_list_count > 0) { previous->os_list_count--; - const bool has_lock = mi_lock_acquire(&previous->subproc->abandoned_os_lock); // this could contend with concurrent OS block abandonment and reclaim from `free` - if (has_lock) { - mi_segment_t* segment = previous->subproc->abandoned_os_list; - // pop from head of the list, a subsequent mark will push at the end (and thus we iterate through os_list_count entries) - if (segment == NULL || mi_arena_segment_os_clear_abandoned(segment, false /* we already have the lock */)) { - mi_lock_release(&previous->subproc->abandoned_os_lock); - return segment; - } - // already abandoned, try again + mi_lock_acquire(&previous->subproc->abandoned_os_lock); // this could contend with concurrent OS block abandonment and reclaim from `free` + mi_segment_t* segment = previous->subproc->abandoned_os_list; + // pop from head of the list, a subsequent mark will push at the end (and thus we iterate through os_list_count entries) + if (segment == NULL || mi_arena_segment_os_clear_abandoned(segment, false /* we already have the lock */)) { mi_lock_release(&previous->subproc->abandoned_os_lock); + return segment; } - else { - _mi_error_message(EFAULT, "failed to acquire abandoned OS list lock during abandoned block visit\n"); - return NULL; - } + // already abandoned, try again + mi_lock_release(&previous->subproc->abandoned_os_lock); } // done mi_assert_internal(previous->os_list_count == 0); diff --git a/src/arena.c b/src/arena.c index 164f3116..86ac5955 100644 --- a/src/arena.c +++ b/src/arena.c @@ -394,8 +394,9 @@ void* _mi_arena_alloc_aligned(size_t size, size_t alignment, size_t align_offset const int numa_node = _mi_os_numa_node(); // current numa node // try to allocate in an arena if the alignment is small enough and the object is not too small (as for heap meta data) - if (!mi_option_is_enabled(mi_option_disallow_arena_alloc) || req_arena_id != _mi_arena_id_none()) { // is arena allocation allowed? - if (size >= MI_ARENA_MIN_OBJ_SIZE && alignment <= MI_SEGMENT_ALIGN && align_offset == 0) { + if (!mi_option_is_enabled(mi_option_disallow_arena_alloc)) { // is arena allocation allowed? + if (size >= MI_ARENA_MIN_OBJ_SIZE && alignment <= MI_SEGMENT_ALIGN && align_offset == 0) + { void* p = mi_arena_try_alloc(numa_node, size, alignment, commit, allow_large, req_arena_id, memid); if (p != NULL) return p; diff --git a/src/init.c b/src/init.c index 3e4da831..68a1d7e2 100644 --- a/src/init.c +++ b/src/init.c @@ -168,8 +168,8 @@ mi_stats_t _mi_stats_main = { MI_STATS_NULL }; #if MI_GUARDED mi_decl_export void mi_heap_guarded_set_sample_rate(mi_heap_t* heap, size_t sample_rate, size_t seed) { heap->guarded_sample_seed = seed; - if (heap->guarded_sample_seed == 0) { - heap->guarded_sample_seed = _mi_heap_random_next(heap); + if (heap->guarded_sample_seed == 0) { + heap->guarded_sample_seed = _mi_heap_random_next(heap); } heap->guarded_sample_rate = sample_rate; if (heap->guarded_sample_rate >= 1) { @@ -187,9 +187,9 @@ void _mi_heap_guarded_init(mi_heap_t* heap) { mi_heap_guarded_set_sample_rate(heap, (size_t)mi_option_get_clamp(mi_option_guarded_sample_rate, 0, LONG_MAX), (size_t)mi_option_get(mi_option_guarded_sample_seed)); - mi_heap_guarded_set_size_bound(heap, + mi_heap_guarded_set_size_bound(heap, (size_t)mi_option_get_clamp(mi_option_guarded_min, 0, LONG_MAX), - (size_t)mi_option_get_clamp(mi_option_guarded_max, 0, LONG_MAX) ); + (size_t)mi_option_get_clamp(mi_option_guarded_max, 0, LONG_MAX) ); } #else mi_decl_export void mi_heap_guarded_set_sample_rate(mi_heap_t* heap, size_t sample_rate, size_t seed) { @@ -257,11 +257,10 @@ void mi_subproc_delete(mi_subproc_id_t subproc_id) { mi_subproc_t* subproc = _mi_subproc_from_id(subproc_id); // check if there are no abandoned segments still.. bool safe_to_delete = false; - if (mi_lock_acquire(&subproc->abandoned_os_lock)) { + mi_lock(&subproc->abandoned_os_lock) { if (subproc->abandoned_os_list == NULL) { safe_to_delete = true; } - mi_lock_release(&subproc->abandoned_os_lock); } if (!safe_to_delete) return; // safe to release @@ -398,7 +397,7 @@ void _mi_tld_init(mi_tld_t* tld, mi_heap_t* bheap) { tld->heap_backing = bheap; tld->heaps = NULL; tld->segments.subproc = &mi_subproc_default; - tld->segments.stats = &tld->stats; + tld->segments.stats = &tld->stats; } // Free the thread local default heap (called from `mi_thread_done`) @@ -599,7 +598,7 @@ static void mi_detect_cpu_features(void) { } #else static void mi_detect_cpu_features(void) { - // nothing + // nothing } #endif