From 0d3c195f376f32ba7de5124d19294a765aaf68f3 Mon Sep 17 00:00:00 2001 From: Daan Leijen Date: Fri, 22 Nov 2019 11:28:55 -0800 Subject: [PATCH 1/4] update stress test with more documentation --- test/test-stress.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/test/test-stress.c b/test/test-stress.c index 6b2fb8c4..b549e1b4 100644 --- a/test/test-stress.c +++ b/test/test-stress.c @@ -5,9 +5,14 @@ terms of the MIT license. -----------------------------------------------------------------------------*/ /* This is a stress test for the allocator, using multiple threads and - transferring objects between threads. This is not a typical workload - but uses a random linear size distribution. Timing can also depend on - (random) thread scheduling. Do not use this test as a benchmark! + transferring objects between threads. It tries to reflect real-world workloads: + - allocation size is distributed linearly in powers of two + - with some fraction extra large (and some extra extra large) + - the allocations are initialized and read again at free + - pointers transfer between threads + - threads are terminated and recreated with some objects surviving in between + - uses deterministic "randomness", but execution can still depend on + (random) thread scheduling. Do not use this test as a benchmark! */ #include @@ -22,13 +27,13 @@ terms of the MIT license. // argument defaults static int THREADS = 32; // more repeatable if THREADS <= #processors static int SCALE = 50; // scaling factor -static int ITER = 10; // N full iterations re-creating all threads +static int ITER = 10; // N full iterations destructing and re-creating all threads // static int THREADS = 8; // more repeatable if THREADS <= #processors // static int SCALE = 100; // scaling factor static bool allow_large_objects = true; // allow very large objects? -static size_t use_one_size = 0; // use single object size of N uintptr_t? +static size_t use_one_size = 0; // use single object size of `N * sizeof(uintptr_t)`? #ifdef USE_STD_MALLOC @@ -185,7 +190,7 @@ int main(int argc, char** argv) { long n = (strtol(argv[3], &end, 10)); if (n > 0) ITER = n; } - printf("start with %d threads with a %d%% load-per-thread and %d iterations\n", THREADS, SCALE, ITER); + printf("Using %d threads with a %d%% load-per-thread and %d iterations\n", THREADS, SCALE, ITER); //int res = mi_reserve_huge_os_pages(4,1); //printf("(reserve huge: %i\n)", res); @@ -204,7 +209,7 @@ int main(int argc, char** argv) { } mi_collect(false); #ifndef NDEBUG - if ((n + 1) % 10 == 0) { printf("- iterations: %3d\n", n + 1); } + if ((n + 1) % 10 == 0) { printf("- iterations left: %3d\n", ITER - n + 1); } #endif } From 4a0d35afd0714f3c8d37957d3a8b384d0591995d Mon Sep 17 00:00:00 2001 From: daan Date: Sat, 23 Nov 2019 11:59:19 -0800 Subject: [PATCH 2/4] improve secure guard page allocation to work with non-eager commit --- src/memory.c | 4 +- src/options.c | 2 +- src/segment.c | 101 +++++++++++++++++++++++++++++++++----------------- 3 files changed, 70 insertions(+), 37 deletions(-) diff --git a/src/memory.c b/src/memory.c index 214bf0d3..b29e18f3 100644 --- a/src/memory.c +++ b/src/memory.c @@ -302,14 +302,14 @@ static void* mi_region_try_alloc(size_t blocks, bool* commit, bool* is_large, bo // no need to commit, but check if already fully committed *commit = mi_bitmap_is_claimed(®ion->commit, 1, blocks, bit_idx); } - mi_assert_internal(mi_bitmap_is_claimed(®ion->commit, 1, blocks, bit_idx)); + mi_assert_internal(!*commit || mi_bitmap_is_claimed(®ion->commit, 1, blocks, bit_idx)); // unreset reset blocks if (mi_bitmap_is_any_claimed(®ion->reset, 1, blocks, bit_idx)) { mi_assert_internal(!info.is_large); mi_assert_internal(!mi_option_is_enabled(mi_option_eager_commit) || *commit); mi_bitmap_unclaim(®ion->reset, 1, blocks, bit_idx); - bool reset_zero; + bool reset_zero = false; _mi_mem_unreset(p, blocks * MI_SEGMENT_SIZE, &reset_zero, tld); if (reset_zero) *is_zero = true; } diff --git a/src/options.c b/src/options.c index 9b6e4cd0..8975a6d3 100644 --- a/src/options.c +++ b/src/options.c @@ -69,7 +69,7 @@ static mi_option_desc_t options[_mi_option_last] = { 0, UNINIT, MI_OPTION(segment_reset) }, // reset segment memory on free (needs eager commit) { 1, UNINIT, MI_OPTION(reset_decommits) }, // reset decommits memory { 0, UNINIT, MI_OPTION(eager_commit_delay) }, // the first N segments per thread are not eagerly committed - { 500,UNINIT, MI_OPTION(reset_delay) }, // reset delay in milli-seconds + { 500, 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 { 16, UNINIT, MI_OPTION(max_errors) } // maximum errors that are output diff --git a/src/segment.c b/src/segment.c index ffba8c0d..0b6501d8 100644 --- a/src/segment.c +++ b/src/segment.c @@ -123,10 +123,18 @@ static bool mi_segment_is_in_free_queue(mi_segment_t* segment, mi_segments_tld_t } #endif -#if (MI_DEBUG>=3) -static size_t mi_segment_pagesize(mi_segment_t* segment) { - return ((size_t)1 << segment->page_shift); +static size_t mi_segment_page_size(mi_segment_t* segment) { + if (segment->capacity > 1) { + mi_assert_internal(segment->page_kind <= MI_PAGE_MEDIUM); + return ((size_t)1 << segment->page_shift); + } + else { + mi_assert_internal(segment->page_kind >= MI_PAGE_LARGE); + return segment->segment_size; + } } + +#if (MI_DEBUG>=3) static bool mi_segment_is_valid(mi_segment_t* segment) { mi_assert_internal(segment != NULL); mi_assert_internal(_mi_ptr_cookie(segment) == segment->cookie); @@ -139,11 +147,47 @@ static bool mi_segment_is_valid(mi_segment_t* segment) { mi_assert_internal(nfree + segment->used == segment->capacity); mi_assert_internal(segment->thread_id == _mi_thread_id() || (segment->thread_id==0)); // or 0 mi_assert_internal(segment->page_kind == MI_PAGE_HUGE || - (mi_segment_pagesize(segment) * segment->capacity == segment->segment_size)); + (mi_segment_page_size(segment) * segment->capacity == segment->segment_size)); return true; } #endif +/* ----------------------------------------------------------- + Guard pages +----------------------------------------------------------- */ + +static void mi_segment_protect_range(void* p, size_t size, bool protect) { + if (protect) { + _mi_mem_protect(p, size); + } + else { + _mi_mem_unprotect(p, size); + } +} + +static void mi_segment_protect(mi_segment_t* segment, bool protect) { + // add/remove guard pages + if (MI_SECURE != 0) { + // in secure mode, we set up a protected page in between the segment info and the page data + const size_t os_page_size = _mi_os_page_size(); + mi_assert_internal((segment->segment_info_size - os_page_size) >= (sizeof(mi_segment_t) + ((segment->capacity - 1) * sizeof(mi_page_t)))); + mi_assert_internal(((uintptr_t)segment + segment->segment_info_size) % os_page_size == 0); + mi_segment_protect_range((uint8_t*)segment + segment->segment_info_size - os_page_size, os_page_size, protect); + if (MI_SECURE <= 1 || segment->capacity == 1) { + // and protect the last (or only) page too + mi_segment_protect_range((uint8_t*)segment + segment->segment_size - os_page_size, os_page_size, protect); + } + else { + // or protect every page + const size_t page_size = mi_segment_page_size(segment); + for (size_t i = 0; i < segment->capacity; i++) { + if (segment->pages[i].is_committed) { + mi_segment_protect_range((uint8_t*)segment + (i+1)*page_size - os_page_size, os_page_size, protect); + } + } + } + } +} /* ----------------------------------------------------------- Page reset @@ -269,15 +313,18 @@ static void mi_segment_os_free(mi_segment_t* segment, size_t segment_size, mi_se mi_segments_track_size(-((long)segment_size),tld); if (MI_SECURE != 0) { mi_assert_internal(!segment->mem_is_fixed); - _mi_mem_unprotect(segment, segment->segment_size); // ensure no more guard pages are set + mi_segment_protect(segment, false); // ensure no more guard pages are set } bool fully_committed = true; bool any_reset = false; for (size_t i = 0; i < segment->capacity; i++) { - const mi_page_t* page = &segment->pages[i]; + mi_page_t* page = &segment->pages[i]; if (!page->is_committed) fully_committed = false; - if (page->is_reset) any_reset = true; + else if (page->is_reset) { + any_reset = true; + // mi_page_unreset(segment, page, 0, tld); + } } _mi_mem_free(segment, segment_size, segment->memid, fully_committed, any_reset, tld->os); } @@ -394,8 +441,7 @@ static mi_segment_t* mi_segment_alloc(size_t required, mi_page_kind_t page_kind, } if (MI_SECURE!=0) { mi_assert_internal(!segment->mem_is_fixed); - // TODO: should we unprotect per page? (with is_protected flag?) - _mi_mem_unprotect(segment, segment->segment_size); // reset protection if the page kind differs + mi_segment_protect(segment, false); // reset protection if the page kind differs } } } @@ -408,7 +454,7 @@ static mi_segment_t* mi_segment_alloc(size_t required, mi_page_kind_t page_kind, if (!commit) { // ensure the initial info is committed bool commit_zero = false; - _mi_mem_commit(segment, info_size, &commit_zero, tld->os); + _mi_mem_commit(segment, pre_size, &commit_zero, tld->os); if (commit_zero) is_zero = true; } segment->memid = memid; @@ -419,25 +465,6 @@ static mi_segment_t* mi_segment_alloc(size_t required, mi_page_kind_t page_kind, mi_assert_internal(segment != NULL && (uintptr_t)segment % MI_SEGMENT_SIZE == 0); if (!pages_still_good) { - // guard pages - if (MI_SECURE != 0) { - // in secure mode, we set up a protected page in between the segment info - // and the page data - mi_assert_internal(info_size == pre_size - _mi_os_page_size() && info_size % _mi_os_page_size() == 0); - _mi_mem_protect((uint8_t*)segment + info_size, (pre_size - info_size)); - const size_t os_page_size = _mi_os_page_size(); - if (MI_SECURE <= 1) { - // and protect the last page too - _mi_mem_protect((uint8_t*)segment + segment_size - os_page_size, os_page_size); - } - else { - // protect every page - for (size_t i = 0; i < capacity; i++) { - _mi_mem_protect((uint8_t*)segment + (i+1)*page_size - os_page_size, os_page_size); - } - } - } - // zero the segment info (but not the `mem` fields) ptrdiff_t ofs = offsetof(mi_segment_t, next); memset((uint8_t*)segment + ofs, 0, info_size - ofs); @@ -465,6 +492,9 @@ static mi_segment_t* mi_segment_alloc(size_t required, mi_page_kind_t page_kind, segment->thread_id = _mi_thread_id(); segment->cookie = _mi_ptr_cookie(segment); _mi_stat_increase(&tld->stats->page_committed, segment->segment_info_size); + + // set protection + mi_segment_protect(segment, true); //fprintf(stderr,"mimalloc: alloc segment at %p\n", (void*)segment); return segment; @@ -525,11 +555,13 @@ static mi_page_t* mi_segment_find_free(mi_segment_t* segment, mi_segments_tld_t* mi_assert_internal(!segment->mem_is_fixed); mi_assert_internal(!page->is_reset); size_t psize; - uint8_t* start = _mi_page_start(segment, page, &psize); + uint8_t* start = mi_segment_raw_page_start(segment, page, &psize); page->is_committed = true; bool is_zero = false; - _mi_mem_commit(start,psize,&is_zero,tld->os); - if (is_zero) page->is_zero_init = true; + const size_t gsize = (MI_SECURE >= 2 ? _mi_os_page_size() : 0); + _mi_mem_commit(start,psize + gsize,&is_zero,tld->os); + if (gsize > 0) { mi_segment_protect_range(start + psize, gsize, true); } + if (is_zero) { page->is_zero_init = true; } } if (page->is_reset) { mi_page_unreset(segment, page, 0, tld); // todo: only unreset the part that was reset? @@ -759,7 +791,7 @@ static mi_page_t* mi_segment_medium_page_alloc(mi_segments_tld_t* tld, mi_os_tld 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); - if (segment == NULL) return NULL; + if (segment == NULL) return NULL; segment->used = 1; mi_page_t* page = &segment->pages[0]; page->segment_in_use = true; @@ -773,7 +805,7 @@ static mi_page_t* mi_segment_huge_page_alloc(size_t size, mi_segments_tld_t* tld { mi_segment_t* segment = mi_segment_alloc(size, MI_PAGE_HUGE, MI_SEGMENT_SHIFT,tld,os_tld); if (segment == NULL) return NULL; - mi_assert_internal(segment->segment_size - segment->segment_info_size >= size); + mi_assert_internal(mi_segment_page_size(segment) - segment->segment_info_size - (2*(MI_SECURE == 0 ? 0 : _mi_os_page_size())) >= size); segment->used = 1; segment->thread_id = 0; // huge pages are immediately abandoned mi_page_t* page = &segment->pages[0]; @@ -800,5 +832,6 @@ mi_page_t* _mi_segment_page_alloc(size_t block_size, mi_segments_tld_t* tld, mi_ page = mi_segment_huge_page_alloc(block_size,tld,os_tld); } mi_assert_expensive(page == NULL || mi_segment_is_valid(_mi_page_segment(page))); + mi_assert_internal(page == NULL || (mi_segment_page_size(_mi_page_segment(page)) - (MI_SECURE == 0 ? 0 : _mi_os_page_size())) >= block_size); return page; } From 727d33b96f9d120d022a9de1bf8b0f39f7645c15 Mon Sep 17 00:00:00 2001 From: daan Date: Sun, 24 Nov 2019 14:40:47 -0800 Subject: [PATCH 3/4] more precise memory reset --- src/memory.c | 16 ++++++++++------ src/segment.c | 50 +++++++++++++++++++++----------------------------- 2 files changed, 31 insertions(+), 35 deletions(-) diff --git a/src/memory.c b/src/memory.c index b29e18f3..9505c98f 100644 --- a/src/memory.c +++ b/src/memory.c @@ -306,15 +306,18 @@ static void* mi_region_try_alloc(size_t blocks, bool* commit, bool* is_large, bo // unreset reset blocks if (mi_bitmap_is_any_claimed(®ion->reset, 1, blocks, bit_idx)) { + // some blocks are still reset mi_assert_internal(!info.is_large); mi_assert_internal(!mi_option_is_enabled(mi_option_eager_commit) || *commit); mi_bitmap_unclaim(®ion->reset, 1, blocks, bit_idx); - bool reset_zero = false; - _mi_mem_unreset(p, blocks * MI_SEGMENT_SIZE, &reset_zero, tld); - if (reset_zero) *is_zero = true; + if (*commit || !mi_option_is_enabled(mi_option_reset_decommits)) { // only if needed + bool reset_zero = false; + _mi_mem_unreset(p, blocks * MI_SEGMENT_SIZE, &reset_zero, tld); + if (reset_zero) *is_zero = true; + } } mi_assert_internal(!mi_bitmap_is_any_claimed(®ion->reset, 1, blocks, bit_idx)); - + #if (MI_DEBUG>=2) if (*commit) { ((uint8_t*)p)[0] = 0; } #endif @@ -409,8 +412,9 @@ void _mi_mem_free(void* p, size_t size, size_t id, bool full_commit, bool any_re } // reset the blocks to reduce the working set. - if (!info.is_large && mi_option_is_enabled(mi_option_segment_reset) && - mi_option_is_enabled(mi_option_eager_commit)) // cannot reset halfway committed segments, use only `option_page_reset` instead + if (!info.is_large && mi_option_is_enabled(mi_option_segment_reset) + && (mi_option_is_enabled(mi_option_eager_commit) || + mi_option_is_enabled(mi_option_reset_decommits))) // cannot reset halfway committed segments, use only `option_page_reset` instead { bool any_unreset; mi_bitmap_claim(®ion->reset, 1, blocks, bit_idx, &any_unreset); diff --git a/src/segment.c b/src/segment.c index 0b6501d8..887248b4 100644 --- a/src/segment.c +++ b/src/segment.c @@ -320,10 +320,10 @@ static void mi_segment_os_free(mi_segment_t* segment, size_t segment_size, mi_se bool any_reset = false; for (size_t i = 0; i < segment->capacity; i++) { mi_page_t* page = &segment->pages[i]; - if (!page->is_committed) fully_committed = false; - else if (page->is_reset) { + if (!page->is_committed) { fully_committed = false; } + if (page->is_reset) { any_reset = true; - // mi_page_unreset(segment, page, 0, tld); + if (mi_option_is_enabled(mi_option_reset_decommits)) { fully_committed = false;} } } _mi_mem_free(segment, segment_size, segment->memid, fully_committed, any_reset, tld->os); @@ -419,7 +419,7 @@ static mi_segment_t* mi_segment_alloc(size_t required, mi_page_kind_t page_kind, // Initialize parameters bool eager_delayed = (page_kind <= MI_PAGE_MEDIUM && tld->count < (size_t)mi_option_get(mi_option_eager_commit_delay)); bool eager = !eager_delayed && mi_option_is_enabled(mi_option_eager_commit); - bool commit = eager || (page_kind >= MI_PAGE_LARGE); + bool commit = eager; // || (page_kind >= MI_PAGE_LARGE); bool pages_still_good = false; bool is_zero = false; @@ -431,18 +431,23 @@ static mi_segment_t* mi_segment_alloc(size_t required, mi_page_kind_t page_kind, } else { + if (MI_SECURE!=0) { + mi_assert_internal(!segment->mem_is_fixed); + mi_segment_protect(segment, false); // reset protection if the page kind differs + } // different page kinds; unreset any reset pages, and unprotect // TODO: optimize cache pop to return fitting pages if possible? for (size_t i = 0; i < segment->capacity; i++) { mi_page_t* page = &segment->pages[i]; if (page->is_reset) { - mi_page_unreset(segment, page, 0, tld); // todo: only unreset the part that was reset? (instead of the full page) + if (!commit && mi_option_is_enabled(mi_option_reset_decommits)) { + page->is_reset = false; + } + else { + mi_page_unreset(segment, page, 0, tld); // todo: only unreset the part that was reset? (instead of the full page) + } } } - if (MI_SECURE!=0) { - mi_assert_internal(!segment->mem_is_fixed); - mi_segment_protect(segment, false); // reset protection if the page kind differs - } } } else { @@ -491,7 +496,7 @@ static mi_segment_t* mi_segment_alloc(size_t required, mi_page_kind_t page_kind, segment->segment_info_size = pre_size; segment->thread_id = _mi_thread_id(); segment->cookie = _mi_ptr_cookie(segment); - _mi_stat_increase(&tld->stats->page_committed, segment->segment_info_size); + // _mi_stat_increase(&tld->stats->page_committed, segment->segment_info_size); // set protection mi_segment_protect(segment, true); @@ -512,18 +517,7 @@ static void mi_segment_free(mi_segment_t* segment, bool force, mi_segments_tld_t mi_assert(segment->next == NULL); mi_assert(segment->prev == NULL); _mi_stat_decrease(&tld->stats->page_committed, segment->segment_info_size); - - // update reset memory statistics - /* - for (uint8_t i = 0; i < segment->capacity; i++) { - mi_page_t* page = &segment->pages[i]; - if (page->is_reset) { - page->is_reset = false; - mi_stat_decrease( tld->stats->reset,mi_page_size(page)); - } - } - */ - + if (!force && mi_segment_cache_push(segment, tld)) { // it is put in our cache } @@ -602,7 +596,7 @@ static void mi_segment_page_clear(mi_segment_t* segment, mi_page_t* page, mi_seg // reset the page memory to reduce memory pressure? // note: must come after setting `segment_in_use` to false - mi_page_reset(segment, page, used_size, tld); + mi_page_reset(segment, page, 0 /*used_size*/, tld); } void _mi_segment_page_free(mi_page_t* page, bool force, mi_segments_tld_t* tld) @@ -792,9 +786,8 @@ static mi_page_t* mi_segment_medium_page_alloc(mi_segments_tld_t* tld, mi_os_tld 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); if (segment == NULL) return NULL; - segment->used = 1; - mi_page_t* page = &segment->pages[0]; - page->segment_in_use = true; + mi_page_t* page = mi_segment_find_free(segment, tld); + mi_assert_internal(page != NULL); #if MI_DEBUG>=2 _mi_segment_page_start(segment, page, sizeof(void*), NULL, NULL)[0] = 0; #endif @@ -806,10 +799,9 @@ static mi_page_t* mi_segment_huge_page_alloc(size_t size, mi_segments_tld_t* tld mi_segment_t* segment = mi_segment_alloc(size, MI_PAGE_HUGE, MI_SEGMENT_SHIFT,tld,os_tld); if (segment == NULL) return NULL; mi_assert_internal(mi_segment_page_size(segment) - segment->segment_info_size - (2*(MI_SECURE == 0 ? 0 : _mi_os_page_size())) >= size); - segment->used = 1; segment->thread_id = 0; // huge pages are immediately abandoned - mi_page_t* page = &segment->pages[0]; - page->segment_in_use = true; + mi_page_t* page = mi_segment_find_free(segment, tld); + mi_assert_internal(page != NULL); return page; } From 4452431b6c66250776200b24465a01e03a393d0a Mon Sep 17 00:00:00 2001 From: daan Date: Sun, 24 Nov 2019 15:25:19 -0800 Subject: [PATCH 4/4] reenable segment cache and fix initial segment commit --- src/segment.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/segment.c b/src/segment.c index 887248b4..9aba8525 100644 --- a/src/segment.c +++ b/src/segment.c @@ -348,7 +348,7 @@ static mi_segment_t* mi_segment_cache_pop(size_t segment_size, mi_segments_tld_t static bool mi_segment_cache_full(mi_segments_tld_t* tld) { - if (tld->count == 1 && tld->cache_count==0) return false; // always cache at least the final segment of a thread + // if (tld->count == 1 && tld->cache_count==0) return false; // always cache at least the final segment of a thread size_t max_cache = mi_option_get(mi_option_segment_cache); if (tld->cache_count < max_cache && tld->cache_count < (1 + (tld->peak_count / MI_SEGMENT_CACHE_FRACTION)) // at least allow a 1 element cache @@ -424,7 +424,7 @@ 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 = NULL; // mi_segment_cache_pop(segment_size, tld); + mi_segment_t* segment = mi_segment_cache_pop(segment_size, tld); if (segment != NULL) { if (page_kind <= MI_PAGE_MEDIUM && segment->page_kind == page_kind && segment->segment_size == segment_size) { pages_still_good = true; @@ -448,6 +448,12 @@ static mi_segment_t* mi_segment_alloc(size_t required, mi_page_kind_t page_kind, } } } + // ensure the initial info is committed + if (segment->capacity < capacity) { + bool commit_zero = false; + _mi_mem_commit(segment, pre_size, &commit_zero, tld->os); + if (commit_zero) is_zero = true; + } } } else {