From 46a9e51f7455254e7ada4fe7623932ef5771c190 Mon Sep 17 00:00:00 2001 From: daan Date: Wed, 10 Jul 2019 07:17:21 -0700 Subject: [PATCH] enable non eager-commit flag --- include/mimalloc-types.h | 1 + src/init.c | 2 +- src/options.c | 2 +- src/os.c | 27 +++++++++++++------ src/segment.c | 57 ++++++++++++++++++++++------------------ 5 files changed, 53 insertions(+), 36 deletions(-) diff --git a/include/mimalloc-types.h b/include/mimalloc-types.h index 145ea9f0..1d48dbda 100644 --- a/include/mimalloc-types.h +++ b/include/mimalloc-types.h @@ -166,6 +166,7 @@ typedef struct mi_page_s { uint8_t segment_idx; // index in the segment `pages` array, `page == &segment->pages[page->segment_idx]` bool segment_in_use:1; // `true` if the segment allocated this page bool is_reset:1; // `true` if the page memory was reset + bool is_committed:1; // `true` if the page virtual memory is committed // layout like this to optimize access in `mi_malloc` and `mi_free` mi_page_flags_t flags; diff --git a/src/init.c b/src/init.c index 95559378..1b925e89 100644 --- a/src/init.c +++ b/src/init.c @@ -11,7 +11,7 @@ terms of the MIT license. A copy of the license can be found in the file // Empty page used to initialize the small free pages array const mi_page_t _mi_page_empty = { - 0, false, false, {0}, + 0, false, false, false, {0}, 0, 0, NULL, 0, 0, // free, used, cookie NULL, 0, {0}, diff --git a/src/options.c b/src/options.c index 9362cb64..528d10df 100644 --- a/src/options.c +++ b/src/options.c @@ -44,7 +44,7 @@ static mi_option_desc_t options[_mi_option_last] = { #else { 0, UNINIT, "secure" }, #endif - { 0, UNINIT, "show_stats" }, + { 1, UNINIT, "show_stats" }, { MI_DEBUG, UNINIT, "show_errors" }, { 0, UNINIT, "verbose" } }; diff --git a/src/os.c b/src/os.c index 29d1dc38..a16527a9 100644 --- a/src/os.c +++ b/src/os.c @@ -382,7 +382,7 @@ static void* mi_os_page_align_areax(bool conservative, void* addr, size_t size, ptrdiff_t diff = (uint8_t*)end - (uint8_t*)start; if (diff <= 0) return NULL; - mi_assert_internal((size_t)diff <= size); + mi_assert_internal((conservative && (size_t)diff <= size) || (!conservative && (size_t)diff >= size)); if (newsize != NULL) *newsize = (size_t)diff; return start; } @@ -391,8 +391,10 @@ static void* mi_os_page_align_area_conservative(void* addr, size_t size, size_t* return mi_os_page_align_areax(true, addr, size, newsize); } -// Commit/Decommit memory. Commit is aligned liberal, while decommit is aligned conservative. -static bool mi_os_commitx(void* addr, size_t size, bool commit, mi_stats_t* stats) { +// Commit/Decommit memory. +// Usuelly commit is aligned liberal, while decommit is aligned conservative. +// (but not for the reset version where we want commit to be conservative as well) +static bool mi_os_commitx(void* addr, size_t size, bool commit, bool conservative, mi_stats_t* stats) { // page align in the range, commit liberally, decommit conservative size_t csize; void* start = mi_os_page_align_areax(!commit, addr, size, &csize); @@ -426,13 +428,18 @@ static bool mi_os_commitx(void* addr, size_t size, bool commit, mi_stats_t* stat } bool _mi_os_commit(void* addr, size_t size, mi_stats_t* stats) { - return mi_os_commitx(addr, size, true, stats); + return mi_os_commitx(addr, size, true, false /* conservative? */, stats); } bool _mi_os_decommit(void* addr, size_t size, mi_stats_t* stats) { - return mi_os_commitx(addr, size, false, stats); + return mi_os_commitx(addr, size, false, true /* conservative? */, stats); } +bool _mi_os_commit_unreset(void* addr, size_t size, mi_stats_t* stats) { + return mi_os_commitx(addr, size, true, true /* conservative? */, stats); +} + + // Signal to the OS that the address range is no longer in use // but may be used later again. This will release physical memory // pages and reduce swapping while keeping the memory committed. @@ -446,6 +453,10 @@ static bool mi_os_resetx(void* addr, size_t size, bool reset, mi_stats_t* stats) else _mi_stat_decrease(&stats->reset, csize); if (!reset) return true; // nothing to do on unreset! + #if MI_DEBUG>1 + memset(start, 0, csize); // pretend it is eagerly reset + #endif + #if defined(_WIN32) // Testing shows that for us (on `malloc-large`) MEM_RESET is 2x faster than DiscardVirtualMemory // (but this is for an access pattern that immediately reuses the memory) @@ -465,7 +476,6 @@ static bool mi_os_resetx(void* addr, size_t size, bool reset, mi_stats_t* stats) DWORD ok = VirtualUnlock(start, csize); if (ok != 0) return false; */ - return true; #else #if defined(MADV_FREE) static int advice = MADV_FREE; @@ -482,8 +492,9 @@ static bool mi_os_resetx(void* addr, size_t size, bool reset, mi_stats_t* stats) _mi_warning_message("madvise reset error: start: 0x%8p, csize: 0x%8zux, errno: %i\n", start, csize, errno); } //mi_assert(err == 0); - return (err == 0); + if (err != 0) return false; #endif + return true; } // Signal to the OS that the address range is no longer in use @@ -501,7 +512,7 @@ bool _mi_os_reset(void* addr, size_t size, mi_stats_t* stats) { bool _mi_os_unreset(void* addr, size_t size, mi_stats_t* stats) { if (mi_option_is_enabled(mi_option_reset_decommits)) { - return _mi_os_commit(addr, size, stats); // re-commit it + return _mi_os_commit_unreset(addr, size, stats); // re-commit it (conservatively!) } else { return mi_os_resetx(addr, size, false, stats); diff --git a/src/segment.c b/src/segment.c index 6d8c5a24..fe356859 100644 --- a/src/segment.c +++ b/src/segment.c @@ -235,7 +235,7 @@ static mi_segment_t* _mi_segment_cache_findx(mi_segments_tld_t* tld, size_t requ if (mi_option_is_enabled(mi_option_secure)) { _mi_os_unprotect(segment, segment->segment_size); } - if (_mi_os_shrink(segment, segment->segment_size, required, tld->stats)) { + if (_mi_os_shrink(segment, segment->segment_size, required, tld->stats)) { // note: double decommit must be (ok on windows) tld->current_size -= segment->segment_size; tld->current_size += required; segment->segment_size = required; @@ -253,18 +253,12 @@ static mi_segment_t* _mi_segment_cache_findx(mi_segments_tld_t* tld, size_t requ return NULL; } +// note: the returned segment might be reset static mi_segment_t* mi_segment_cache_find(mi_segments_tld_t* tld, size_t required) { - mi_segment_t* segment = _mi_segment_cache_findx(tld,required,false); - if (segment != NULL && - mi_option_is_enabled(mi_option_eager_commit) && - (mi_option_is_enabled(mi_option_cache_reset) || mi_option_is_enabled(mi_option_page_reset))) - { - // ensure the memory is available - _mi_os_unreset((uint8_t*)segment + segment->segment_info_size, segment->segment_size - segment->segment_info_size, tld->stats); - } - return segment; + return _mi_segment_cache_findx(tld,required,false); } +// note: the returned segment might be reset static mi_segment_t* mi_segment_cache_evict(mi_segments_tld_t* tld) { // TODO: random eviction instead? return _mi_segment_cache_findx(tld, 0, true /* from the end */); @@ -359,9 +353,16 @@ static mi_segment_t* mi_segment_alloc( size_t required, mi_page_kind_t page_kind protection_still_good = true; // otherwise, the guard pages are still in place } } - if (page_kind != MI_PAGE_SMALL && !mi_option_is_enabled(mi_option_eager_commit) && - (mi_option_is_enabled(mi_option_cache_reset) || mi_option_is_enabled(mi_option_page_reset))) { - _mi_os_commit(segment, segment->segment_size, tld->stats); + if (!mi_option_is_enabled(mi_option_eager_commit)) { + if (page_kind != MI_PAGE_SMALL) { + _mi_os_commit(segment, segment->segment_size, tld->stats); + } + else { + // ok, commit (and unreset) on demand again + } + } + else if (mi_option_is_enabled(mi_option_cache_reset) || mi_option_is_enabled(mi_option_page_reset)) { + _mi_os_unreset(segment, segment->segment_size, tld->stats); } } // and otherwise allocate it from the OS @@ -369,11 +370,11 @@ static mi_segment_t* mi_segment_alloc( size_t required, mi_page_kind_t page_kind segment = (mi_segment_t*)_mi_os_alloc_aligned(segment_size, MI_SEGMENT_SIZE, commit, os_tld); if (segment == NULL) return NULL; mi_segments_track_size((long)segment_size,tld); + if (!commit) { + _mi_os_commit(segment, info_size, tld->stats); // always commit start of the segment + } } mi_assert_internal(segment != NULL && (uintptr_t)segment % MI_SEGMENT_SIZE == 0); - if (!commit) { - _mi_os_commit(segment,info_size,tld->stats); - } memset(segment, 0, info_size); if (mi_option_is_enabled(mi_option_secure) && !protection_still_good) { @@ -403,7 +404,8 @@ static mi_segment_t* mi_segment_alloc( size_t required, mi_page_kind_t page_kind segment->cookie = _mi_ptr_cookie(segment); for (uint8_t i = 0; i < segment->capacity; i++) { segment->pages[i].segment_idx = i; - segment->pages[i].is_reset = !commit; + segment->pages[i].is_reset = false; + segment->pages[i].is_committed = commit; } _mi_stat_increase(&tld->stats->page_committed, segment->segment_info_size); //fprintf(stderr,"mimalloc: alloc segment at %p\n", (void*)segment); @@ -477,18 +479,18 @@ static mi_page_t* mi_segment_find_free(mi_segment_t* segment, mi_stats_t* stats) for (size_t i = 0; i < segment->capacity; i++) { mi_page_t* page = &segment->pages[i]; if (!page->segment_in_use) { - if (page->is_reset) { + if (page->is_reset || !page->is_committed) { size_t psize; uint8_t* start = _mi_page_start(segment, page, &psize); - page->is_reset = false; - if (mi_option_is_enabled(mi_option_eager_commit)) { - _mi_os_unreset(start, psize, stats); - } - else { - // note we could allow both lazy commit, and page level reset if we add a `is_commit` flag... - // for now we use commit for both - _mi_os_commit(start, psize, stats); + mi_assert_internal(!(page->is_reset && !page->is_committed)); + if (!page->is_committed) { + page->is_committed = true; + _mi_os_commit(start,psize,stats); } + if (page->is_reset) { + page->is_reset = false; + _mi_os_unreset(start, psize, stats); + } } return page; } @@ -508,6 +510,7 @@ static void mi_segment_page_clear(mi_segment_t* segment, mi_page_t* page, mi_sta UNUSED(stats); mi_assert_internal(page->segment_in_use); mi_assert_internal(mi_page_all_free(page)); + mi_assert_internal(page->is_committed); size_t inuse = page->capacity * page->block_size; _mi_stat_decrease(&stats->page_committed, inuse); _mi_stat_decrease(&stats->pages, 1); @@ -523,10 +526,12 @@ static void mi_segment_page_clear(mi_segment_t* segment, mi_page_t* page, mi_sta // zero the page data uint8_t idx = page->segment_idx; // don't clear the index bool is_reset = page->is_reset; // don't clear the reset flag + bool is_committed = page->is_committed; // don't clear the commit flag memset(page, 0, sizeof(*page)); page->segment_idx = idx; page->segment_in_use = false; page->is_reset = is_reset; + page->is_committed = is_committed; segment->used--; }