From 515047b676c43b0de8a7b547716500aeea69793a Mon Sep 17 00:00:00 2001 From: Daan Date: Wed, 5 Feb 2025 20:55:21 -0800 Subject: [PATCH] improve free on macos --- include/mimalloc/internal.h | 4 ++-- src/free.c | 31 ++++++++++++++++++++----------- src/page-map.c | 8 ++++---- 3 files changed, 26 insertions(+), 17 deletions(-) diff --git a/include/mimalloc/internal.h b/include/mimalloc/internal.h index 92f02788..25e30f10 100644 --- a/include/mimalloc/internal.h +++ b/include/mimalloc/internal.h @@ -492,7 +492,7 @@ static inline mi_page_t* _mi_unchecked_ptr_page(const void* p) { // 2-level page map: // double indirection, but low commit and low virtual reserve. // -// the page-map is usually 4 MiB and points to sub maps of 64 KiB. +// the page-map is usually 4 MiB (for 48 bits virtual addresses) and points to sub maps of 64 KiB. // the page-map is committed on-demand (in 64 KiB parts) (and sub-maps are committed on-demand as well) // one sub page-map = 64 KiB => covers 2^(16-3) * 2^16 = 2^29 = 512 MiB address space // the page-map needs 48-(16+13) = 19 bits => 2^19 sub map pointers = 4 MiB size. @@ -519,7 +519,7 @@ static inline mi_page_t* _mi_checked_ptr_page(const void* p) { size_t sub_idx; const size_t idx = _mi_page_map_index(p, &sub_idx); mi_page_t** const sub = _mi_page_map[idx]; - if mi_unlikely(sub == NULL) return NULL; + if mi_unlikely(sub == NULL) return (mi_page_t*)&_mi_page_empty; return sub[sub_idx]; } diff --git a/src/free.c b/src/free.c index c584e150..266faad8 100644 --- a/src/free.c +++ b/src/free.c @@ -123,6 +123,10 @@ static void mi_decl_noinline mi_free_generic_local(mi_page_t* page, void* p) mi_ // free a pointer owned by another thread (page parameter comes first for better codegen) static void mi_decl_noinline mi_free_generic_mt(mi_page_t* page, void* p) mi_attr_noexcept { if (p==NULL) return; // a NULL pointer is seen as abandoned (tid==0) with a full flag set + #if !MI_PAGE_MAP_FLAT + if (page==&_mi_page_empty) return; // an invalid pointer may lead to using the empty page + #endif + mi_assert_internal(p!=NULL && page != NULL && page != &_mi_page_empty); mi_block_t* const block = _mi_page_ptr_unalign(page, p); // don't check `has_aligned` flag to avoid a race (issue #865) mi_block_check_unguard(page, block, p); mi_free_block_mt(page, block); @@ -135,10 +139,9 @@ void mi_decl_noinline _mi_free_generic(mi_page_t* page, bool is_local, void* p) } -// Get the segment data belonging to a pointer -// This is just a single `and` in release mode but does further checks in debug mode -// (and secure mode) to see if this was a valid pointer. -static inline mi_page_t* mi_checked_ptr_page(const void* p, const char* msg) +// Get the page belonging to a pointer +// Does further checks in debug mode to see if this was a valid pointer. +static inline mi_page_t* mi_validate_ptr_page(const void* p, const char* msg) { MI_UNUSED_RELEASE(msg); #if MI_DEBUG @@ -146,9 +149,14 @@ static inline mi_page_t* mi_checked_ptr_page(const void* p, const char* msg) _mi_error_message(EINVAL, "%s: invalid (unaligned) pointer: %p\n", msg, p); return NULL; } - mi_page_t* const page = _mi_safe_ptr_page(p); - if (page == NULL && p != NULL) { - _mi_error_message(EINVAL, "%s: invalid pointer: %p\n", msg, p); + mi_page_t* page = _mi_safe_ptr_page(p); + if (page == NULL) { + if (p != NULL) { + _mi_error_message(EINVAL, "%s: invalid pointer: %p\n", msg, p); + } + #if !MI_PAGE_MAP_FLAT + page = (mi_page_t*)&_mi_page_empty; + #endif } return page; #else @@ -160,12 +168,13 @@ static inline mi_page_t* mi_checked_ptr_page(const void* p, const char* msg) // Fast path written carefully to prevent register spilling on the stack void mi_free(void* p) mi_attr_noexcept { - mi_page_t* const page = mi_checked_ptr_page(p,"mi_free"); + mi_page_t* const page = mi_validate_ptr_page(p,"mi_free"); - #if MI_PAGE_MAP_FLAT // if not flat, NULL will point to `_mi_page_empty` and get to `mi_free_generic_mt` + #if MI_PAGE_MAP_FLAT // if not flat, p==NULL leads to `_mi_page_empty` which leads to `mi_free_generic_mt` if mi_unlikely(page==NULL) return; #endif - + mi_assert_internal(page!=NULL); + const mi_threadid_t xtid = (_mi_prim_thread_id() ^ mi_page_xthread_id(page)); if mi_likely(xtid == 0) { // `tid == mi_page_thread_id(page) && mi_page_flags(page) == 0` // thread-local, aligned, and not a full page @@ -283,7 +292,7 @@ static size_t mi_decl_noinline mi_page_usable_aligned_size_of(const mi_page_t* p } static inline size_t _mi_usable_size(const void* p, const char* msg) mi_attr_noexcept { - const mi_page_t* const page = mi_checked_ptr_page(p,msg); + const mi_page_t* const page = mi_validate_ptr_page(p,msg); if mi_unlikely(page==NULL) return 0; if mi_likely(!mi_page_has_aligned(page)) { const mi_block_t* block = (const mi_block_t*)p; diff --git a/src/page-map.c b/src/page-map.c index 2b610935..74c22e90 100644 --- a/src/page-map.c +++ b/src/page-map.c @@ -206,7 +206,7 @@ bool _mi_page_map_init(void) { if (!mi_page_map_memid.initially_committed) { _mi_os_commit(&_mi_page_map[0], os_page_size, NULL); // commit first part of the map } - _mi_page_map[0] = (mi_page_t**)((uint8_t*)_mi_page_map + page_map_size); // we reserved 2 subs at the end already + _mi_page_map[0] = (mi_page_t**)((uint8_t*)_mi_page_map + page_map_size); // we reserved 2 sub maps at the end already if (!mi_page_map_memid.initially_committed) { _mi_os_commit(_mi_page_map[0], os_page_size, NULL); // only first OS page } @@ -315,10 +315,10 @@ void _mi_page_map_unregister_range(void* start, size_t size) { mi_page_map_set_range(NULL, idx, sub_idx, slice_count); // todo: avoid committing if not already committed? } -// Return the empty page for the NULL pointer to match the behaviour of `_mi_ptr_page` +// Return NULL for invalid pointers mi_page_t* _mi_safe_ptr_page(const void* p) { + if (p==NULL) return NULL; if mi_unlikely(p >= mi_page_map_max_address) return NULL; - if (p == NULL) return (mi_page_t*)&_mi_page_empty; // to match `_mi_ptr_page` (see `mi_free` as well) size_t sub_idx; const size_t idx = _mi_page_map_index(p,&sub_idx); if mi_unlikely(!mi_page_map_is_committed(idx,NULL)) return NULL; @@ -328,7 +328,7 @@ mi_page_t* _mi_safe_ptr_page(const void* p) { } mi_decl_nodiscard mi_decl_export bool mi_is_in_heap_region(const void* p) mi_attr_noexcept { - return (p != NULL && _mi_safe_ptr_page(p) != NULL); + return (_mi_safe_ptr_page(p) != NULL); } #endif