From 28d4ec4c5ad39bc9459cd432c34f8d9234b51431 Mon Sep 17 00:00:00 2001 From: daan Date: Mon, 28 Oct 2019 13:14:14 -0700 Subject: [PATCH 1/3] fix statistics accounting of huge pages --- src/alloc.c | 10 +++++++++- src/page.c | 3 ++- test/test-stress.c | 7 ++++++- 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/src/alloc.c b/src/alloc.c index 916b1f32..f8dab24d 100644 --- a/src/alloc.c +++ b/src/alloc.c @@ -192,7 +192,15 @@ static mi_decl_noinline void _mi_free_block_mt(mi_page_t* page, mi_block_t* bloc page->free = block; page->used--; page->is_zero = false; - _mi_segment_page_free(page,true,&heap->tld->segments); + mi_assert(page->used == 0); + mi_tld_t* tld = heap->tld; + if (page->block_size > MI_HUGE_OBJ_SIZE_MAX) { + _mi_stat_decrease(&tld->stats.giant, page->block_size); + } + else { + _mi_stat_decrease(&tld->stats.huge, page->block_size); + } + _mi_segment_page_free(page,true,&tld->segments); } return; } diff --git a/src/page.c b/src/page.c index 77d98f11..b71be522 100644 --- a/src/page.c +++ b/src/page.c @@ -370,6 +370,7 @@ void _mi_page_free(mi_page_t* page, mi_page_queue_t* pq, bool force) { mi_page_set_has_aligned(page, false); // account for huge pages here + // (note: no longer necessary as huge pages are always abandoned) if (page->block_size > MI_LARGE_OBJ_SIZE_MAX) { if (page->block_size > MI_HUGE_OBJ_SIZE_MAX) { _mi_stat_decrease(&page->heap->tld->stats.giant, page->block_size); @@ -378,7 +379,7 @@ void _mi_page_free(mi_page_t* page, mi_page_queue_t* pq, bool force) { _mi_stat_decrease(&page->heap->tld->stats.huge, page->block_size); } } - + // remove from the page list // (no need to do _mi_heap_delayed_free first as all blocks are already free) mi_segments_tld_t* segments_tld = &page->heap->tld->segments; diff --git a/test/test-stress.c b/test/test-stress.c index 354e2b07..bb428072 100644 --- a/test/test-stress.c +++ b/test/test-stress.c @@ -158,6 +158,7 @@ int main(int argc, char** argv) { //printf("(reserve huge: %i\n)", res); //bench_start_program(); + mi_stats_reset(); memset((void*)transfer, 0, TRANSFERS*sizeof(void*)); run_os_threads(THREADS); for (int i = 0; i < TRANSFERS; i++) { @@ -165,7 +166,6 @@ int main(int argc, char** argv) { } #ifndef NDEBUG mi_collect(false); - mi_collect(true); #endif mi_stats_print(NULL); //bench_end_program(); @@ -191,6 +191,11 @@ static void run_os_threads(size_t nthreads) { for (size_t i = 0; i < nthreads; i++) { WaitForSingleObject(thandles[i], INFINITE); } + for (size_t i = 0; i < nthreads; i++) { + CloseHandle(thandles[i]); + } + free(tids); + free(thandles); } static void* atomic_exchange_ptr(volatile void** p, void* newval) { From 081e2d1eb6d517f1949b3245d37f758e8a27ac3f Mon Sep 17 00:00:00 2001 From: daan Date: Mon, 28 Oct 2019 13:43:42 -0700 Subject: [PATCH 2/3] fix statistics display --- include/mimalloc-types.h | 6 +++--- src/init.c | 4 ++-- src/os.c | 4 ++-- src/page.c | 2 +- src/stats.c | 36 ++++++++++++++++++++++++------------ 5 files changed, 32 insertions(+), 20 deletions(-) diff --git a/include/mimalloc-types.h b/include/mimalloc-types.h index 00a83839..2e5d481b 100644 --- a/include/mimalloc-types.h +++ b/include/mimalloc-types.h @@ -340,14 +340,14 @@ typedef struct mi_stats_s { mi_stat_count_t page_committed; mi_stat_count_t segments_abandoned; mi_stat_count_t pages_abandoned; - mi_stat_count_t pages_extended; - mi_stat_count_t mmap_calls; - mi_stat_count_t commit_calls; mi_stat_count_t threads; mi_stat_count_t huge; mi_stat_count_t giant; mi_stat_count_t malloc; mi_stat_count_t segments_cache; + mi_stat_counter_t pages_extended; + mi_stat_counter_t mmap_calls; + mi_stat_counter_t commit_calls; mi_stat_counter_t page_no_retire; mi_stat_counter_t searches; mi_stat_counter_t huge_count; diff --git a/src/init.c b/src/init.c index 6514ce53..d361de3a 100644 --- a/src/init.c +++ b/src/init.c @@ -64,8 +64,8 @@ const mi_page_t _mi_page_empty = { MI_STAT_COUNT_NULL(), MI_STAT_COUNT_NULL(), \ MI_STAT_COUNT_NULL(), MI_STAT_COUNT_NULL(), \ MI_STAT_COUNT_NULL(), MI_STAT_COUNT_NULL(), \ - MI_STAT_COUNT_NULL(), MI_STAT_COUNT_NULL(), \ - MI_STAT_COUNT_NULL(), MI_STAT_COUNT_NULL(), \ + MI_STAT_COUNT_NULL(), \ + { 0, 0 }, { 0, 0 }, { 0, 0 }, \ { 0, 0 }, { 0, 0 }, { 0, 0 }, { 0, 0 } \ MI_STAT_COUNT_END_NULL() diff --git a/src/os.c b/src/os.c index 0cd3a1ab..8f5afc5b 100644 --- a/src/os.c +++ b/src/os.c @@ -477,7 +477,7 @@ static void* mi_os_mem_alloc(size_t size, size_t try_alignment, bool commit, boo int protect_flags = (commit ? (PROT_WRITE | PROT_READ) : PROT_NONE); p = mi_unix_mmap(NULL, size, try_alignment, protect_flags, false, allow_large, is_large); #endif - _mi_stat_increase(&stats->mmap_calls, 1); + mi_stat_counter_increase(stats->mmap_calls, 1); if (p != NULL) { _mi_stat_increase(&stats->reserved, size); if (commit) { _mi_stat_increase(&stats->committed, size); } @@ -632,7 +632,7 @@ static bool mi_os_commitx(void* addr, size_t size, bool commit, bool conservativ int err = 0; if (commit) { _mi_stat_increase(&stats->committed, csize); - _mi_stat_increase(&stats->commit_calls, 1); + _mi_stat_counter_increase(&stats->commit_calls, 1); } else { _mi_stat_decrease(&stats->committed, csize); diff --git a/src/page.c b/src/page.c index b71be522..2a48c64b 100644 --- a/src/page.c +++ b/src/page.c @@ -531,7 +531,7 @@ static void mi_page_extend_free(mi_heap_t* heap, mi_page_t* page, mi_stats_t* st size_t page_size; _mi_page_start(_mi_page_segment(page), page, &page_size); - mi_stat_increase(stats->pages_extended, 1); + mi_stat_counter_increase(stats->pages_extended, 1); // calculate the extend count size_t extend = page->reserved - page->capacity; diff --git a/src/stats.c b/src/stats.c index 37a7bde4..50bd029d 100644 --- a/src/stats.c +++ b/src/stats.c @@ -95,15 +95,17 @@ static void mi_stats_add(mi_stats_t* stats, const mi_stats_t* src) { mi_stat_add(&stats->pages_abandoned, &src->pages_abandoned, 1); mi_stat_add(&stats->segments_abandoned, &src->segments_abandoned, 1); - mi_stat_add(&stats->mmap_calls, &src->mmap_calls, 1); - mi_stat_add(&stats->commit_calls, &src->commit_calls, 1); mi_stat_add(&stats->threads, &src->threads, 1); - mi_stat_add(&stats->pages_extended, &src->pages_extended, 1); mi_stat_add(&stats->malloc, &src->malloc, 1); mi_stat_add(&stats->segments_cache, &src->segments_cache, 1); mi_stat_add(&stats->huge, &src->huge, 1); mi_stat_add(&stats->giant, &src->giant, 1); + + mi_stat_counter_add(&stats->pages_extended, &src->pages_extended, 1); + mi_stat_counter_add(&stats->mmap_calls, &src->mmap_calls, 1); + mi_stat_counter_add(&stats->commit_calls, &src->commit_calls, 1); + mi_stat_counter_add(&stats->page_no_retire, &src->page_no_retire, 1); mi_stat_counter_add(&stats->searches, &src->searches, 1); mi_stat_counter_add(&stats->huge_count, &src->huge_count, 1); @@ -121,6 +123,9 @@ static void mi_stats_add(mi_stats_t* stats, const mi_stats_t* src) { Display statistics ----------------------------------------------------------- */ +// unit > 0 : size in binary bytes +// unit == 0: count as decimal +// unit < 0 : count in binary static void mi_printf_amount(int64_t n, int64_t unit, mi_output_fun* out, const char* fmt) { char buf[32]; int len = 32; @@ -165,17 +170,24 @@ static void mi_stat_print(const mi_stat_count_t* stat, const char* msg, int64_t _mi_fprintf(out, " ok\n"); } else if (unit<0) { - mi_print_amount(stat->peak, 1, out); - mi_print_amount(stat->allocated, 1, out); - mi_print_amount(stat->freed, 1, out); - mi_print_amount(-unit, 1, out); - mi_print_count((stat->allocated / -unit), 0, out); + mi_print_amount(stat->peak, -1, out); + mi_print_amount(stat->allocated, -1, out); + mi_print_amount(stat->freed, -1, out); + if (unit==-1) { + _mi_fprintf(out, "%22s", ""); + } + else { + mi_print_amount(-unit, 1, out); + mi_print_count((stat->allocated / -unit), 0, out); + } if (stat->allocated > stat->freed) _mi_fprintf(out, " not all freed!\n"); else _mi_fprintf(out, " ok\n"); } else { + mi_print_amount(stat->peak, 1, out); + mi_print_amount(stat->allocated, 1, out); _mi_fprintf(out, "\n"); } } @@ -247,11 +259,11 @@ static void _mi_stats_print(mi_stats_t* stats, double secs, mi_output_fun* out) mi_stat_print(&stats->segments_cache, "-cached", -1, out); mi_stat_print(&stats->pages, "pages", -1, out); mi_stat_print(&stats->pages_abandoned, "-abandoned", -1, out); - mi_stat_print(&stats->pages_extended, "-extended", 0, out); + mi_stat_counter_print(&stats->pages_extended, "-extended", out); mi_stat_counter_print(&stats->page_no_retire, "-noretire", out); - mi_stat_print(&stats->mmap_calls, "mmaps", 0, out); - mi_stat_print(&stats->commit_calls, "commits", 0, out); - mi_stat_print(&stats->threads, "threads", 0, out); + mi_stat_counter_print(&stats->mmap_calls, "mmaps", out); + mi_stat_counter_print(&stats->commit_calls, "commits", out); + mi_stat_print(&stats->threads, "threads", -1, out); mi_stat_counter_print_avg(&stats->searches, "searches", out); if (secs >= 0.0) _mi_fprintf(out, "%10s: %9.3f s\n", "elapsed", secs); From 87bdfbb9b6b0a869f9ce8e76fd4fa580bb816840 Mon Sep 17 00:00:00 2001 From: daan Date: Mon, 28 Oct 2019 14:12:06 -0700 Subject: [PATCH 3/3] use more conservative retire strategy --- src/page.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/page.c b/src/page.c index 2a48c64b..5a186727 100644 --- a/src/page.c +++ b/src/page.c @@ -407,16 +407,18 @@ void _mi_page_retire(mi_page_t* page) { // (or we end up retiring and re-allocating most of the time) // NOTE: refine this more: we should not retire if this // is the only page left with free blocks. It is not clear - // how to check this efficiently though... for now we just check - // if its neighbours are almost fully used. + // how to check this efficiently though... + // for now, we don't retire if it is the only page left of this size class. + mi_page_queue_t* pq = mi_page_queue_of(page); if (mi_likely(page->block_size <= (MI_SMALL_SIZE_MAX/4))) { - if (mi_page_mostly_used(page->prev) && mi_page_mostly_used(page->next)) { + // if (mi_page_mostly_used(page->prev) && mi_page_mostly_used(page->next)) { + if (pq->last==page && pq->first==page) { mi_stat_counter_increase(_mi_stats_main.page_no_retire,1); return; // dont't retire after all } } - _mi_page_free(page, mi_page_queue_of(page), false); + _mi_page_free(page, pq, false); }