From 0043e68d4c54a305d84ead95cae27a730540451b Mon Sep 17 00:00:00 2001 From: Qi Wang Date: Tue, 20 Aug 2019 18:14:18 -0700 Subject: [PATCH] Track low_water == -1 case explicitly. The -1 value of low_water indicates if the cache has been depleted and refilled. Track the status explicitly in the tcache struct. This allows the fast path to check if (cur_ptr > low_water), instead of >=, which avoids reaching slow path when the last item is allocated. --- include/jemalloc/internal/cache_bin.h | 15 +++++++-------- include/jemalloc/internal/tcache_structs.h | 2 ++ src/arena.c | 2 +- src/tcache.c | 19 ++++++++++++------- test/unit/cache_bin.c | 2 +- 5 files changed, 23 insertions(+), 17 deletions(-) diff --git a/include/jemalloc/internal/cache_bin.h b/include/jemalloc/internal/cache_bin.h index 775eb3fa..7ec1ccbf 100644 --- a/include/jemalloc/internal/cache_bin.h +++ b/include/jemalloc/internal/cache_bin.h @@ -56,7 +56,7 @@ struct cache_bin_s { * 2) full points to the top of the stack (i.e. ncached == ncached_max), * which is compared against on free_fastpath to check "is_full". * 3) low_water indicates a low water mark of ncached. - * Range of low_water is [cur, empty + 1], i.e. values of [ncached, -1]. + * Range of low_water is [cur, empty], i.e. values of [ncached, 0]. * * The empty position (ncached == 0) is derived via full + ncached_max * and not accessed in the common case (guarded behind low_water). @@ -87,9 +87,7 @@ struct cache_bin_s { cache_bin_stats_t tstats; /* * Points to the first item that hasn't been used since last GC, to - * track the low water mark (min # of cached). It may point to - * empty_position + 1, which indicates the cache has been depleted and - * refilled (low_water == -1). + * track the low water mark (min # of cached). */ uint32_t low_water_position; /* @@ -165,7 +163,7 @@ cache_bin_low_water_get(cache_bin_t *bin, szind_t ind) { cache_bin_sz_t ncached_max = cache_bin_ncached_max_get(ind); cache_bin_sz_t low_water = ncached_max - (bin->low_water_position - bin->full_position) / sizeof(void *); - assert(low_water >= -1 && low_water <= ncached_max); + assert(low_water >= 0 && low_water <= ncached_max); assert(low_water <= cache_bin_ncached_get(bin, ind)); assert(bin->low_water_position >= bin->cur_ptr.lowbits); @@ -200,16 +198,17 @@ cache_bin_alloc_easy(cache_bin_t *bin, bool *success, cache_bin_sz_t ind) { * branch. This also avoids accessing tcache_bin_info (which is on a * separate cacheline / page) in the common case. */ - if (unlikely(bin->cur_ptr.lowbits >= bin->low_water_position)) { - bin->low_water_position = bin->cur_ptr.lowbits; + if (unlikely(bin->cur_ptr.lowbits > bin->low_water_position)) { uint32_t empty_position = bin->full_position + tcache_bin_info[ind].stack_size; - if (bin->cur_ptr.lowbits > empty_position) { + if (unlikely(bin->cur_ptr.lowbits > empty_position)) { + /* Over-allocated; revert. */ bin->cur_ptr.ptr--; assert(bin->cur_ptr.lowbits == empty_position); *success = false; return NULL; } + bin->low_water_position = bin->cur_ptr.lowbits; } /* diff --git a/include/jemalloc/internal/tcache_structs.h b/include/jemalloc/internal/tcache_structs.h index 172ef904..008b1f73 100644 --- a/include/jemalloc/internal/tcache_structs.h +++ b/include/jemalloc/internal/tcache_structs.h @@ -51,6 +51,8 @@ struct tcache_s { szind_t next_gc_bin; /* For small bins, fill (ncached_max >> lg_fill_div). */ uint8_t lg_fill_div[SC_NBINS]; + /* For small bins, whether has been refilled since last GC. */ + bool bin_refilled[SC_NBINS]; /* * We put the cache bins for large size classes at the end of the * struct, since some of them might not get used. This might end up diff --git a/src/arena.c b/src/arena.c index b383befe..aa707f43 100644 --- a/src/arena.c +++ b/src/arena.c @@ -1383,10 +1383,10 @@ arena_tcache_fill_small(tsdn_t *tsdn, arena_t *arena, tcache_t *tcache, unsigned i, nfill, cnt; assert(cache_bin_ncached_get(tbin, binind) == 0); - if (config_prof && arena_prof_accum(tsdn, arena, prof_accumbytes)) { prof_idump(tsdn); } + tcache->bin_refilled[binind] = true; unsigned binshard; bin_t *bin = arena_bin_choose_lock(tsdn, arena, binind, &binshard); diff --git a/src/tcache.c b/src/tcache.c index 2594a029..8f89c55f 100644 --- a/src/tcache.c +++ b/src/tcache.c @@ -17,8 +17,8 @@ cache_bin_info_t *tcache_bin_info; /* * For the total bin stack region (per tcache), reserve 2 more slots so that 1) * the empty position can be safely read on the fast path before checking - * "is_empty"; and 2) the low_water == -1 case can go beyond the empty position - * by 1 step safely (i.e. no overflow). + * "is_empty"; and 2) the cur_ptr can go beyond the empty position by 1 step + * safely on the fast path (i.e. no overflow). */ static const unsigned total_stack_padding = sizeof(void *) * 2; @@ -49,12 +49,14 @@ tcache_salloc(tsdn_t *tsdn, const void *ptr) { void tcache_event_hard(tsd_t *tsd, tcache_t *tcache) { szind_t binind = tcache->next_gc_bin; - cache_bin_t *tbin; + bool is_small; if (binind < SC_NBINS) { tbin = tcache_small_bin_get(tcache, binind); + is_small = true; } else { tbin = tcache_large_bin_get(tcache, binind); + is_small = false; } cache_bin_sz_t low_water = cache_bin_low_water_get(tbin, binind); @@ -63,7 +65,8 @@ tcache_event_hard(tsd_t *tsd, tcache_t *tcache) { /* * Flush (ceiling) 3/4 of the objects below the low water mark. */ - if (binind < SC_NBINS) { + if (is_small) { + assert(!tcache->bin_refilled[binind]); tcache_bin_flush_small(tsd, tcache, tbin, binind, ncached - low_water + (low_water >> 2)); /* @@ -78,15 +81,16 @@ tcache_event_hard(tsd_t *tsd, tcache_t *tcache) { tcache_bin_flush_large(tsd, tcache, tbin, binind, ncached - low_water + (low_water >> 2)); } - } else if (low_water < 0) { - assert(low_water == -1); + } else if (is_small && tcache->bin_refilled[binind]) { + assert(low_water == 0); /* * Increase fill count by 2X for small bins. Make sure * lg_fill_div stays greater than 0. */ - if (binind < SC_NBINS && tcache->lg_fill_div[binind] > 1) { + if (tcache->lg_fill_div[binind] > 1) { tcache->lg_fill_div[binind]--; } + tcache->bin_refilled[binind] = false; } tbin->low_water_position = tbin->cur_ptr.lowbits; @@ -472,6 +476,7 @@ tcache_init(tsd_t *tsd, tcache_t *tcache, void *avail_stack) { uintptr_t stack_cur = (uintptr_t)avail_stack; for (; i < SC_NBINS; i++) { tcache->lg_fill_div[i] = 1; + tcache->bin_refilled[i] = false; cache_bin_t *bin = tcache_small_bin_get(tcache, i); tcache_bin_init(bin, i, &stack_cur); } diff --git a/test/unit/cache_bin.c b/test/unit/cache_bin.c index d8900417..f469b8da 100644 --- a/test/unit/cache_bin.c +++ b/test/unit/cache_bin.c @@ -23,7 +23,7 @@ TEST_BEGIN(test_cache_bin) { bool success; void *ret = cache_bin_alloc_easy(bin, &success, 0); assert_false(success, "Empty cache bin should not alloc"); - assert_true(cache_bin_low_water_get(bin, 0) == - 1, + assert_true(cache_bin_low_water_get(bin, 0) == 0, "Incorrect low water mark"); cache_bin_ncached_set(bin, 0, 0);