From e1dcc557d68cfa1c7f1fab6c84a9e44e1d97e1d4 Mon Sep 17 00:00:00 2001 From: David Goldblatt Date: Fri, 28 Feb 2020 18:55:33 -0800 Subject: [PATCH] Cache bin: Only take the relevant cache_bin_info_t Previously, we took an array of cache_bin_info_ts and an index, and dereferenced ourselves. But infos for other cache_bins aren't relevant to any particular cache bin, so that should be the caller's job. --- include/jemalloc/internal/cache_bin.h | 71 +++++++++------------- include/jemalloc/internal/tcache_inlines.h | 8 +-- src/arena.c | 16 ++--- src/tcache.c | 30 ++++----- test/unit/cache_bin.c | 26 ++++---- 5 files changed, 70 insertions(+), 81 deletions(-) diff --git a/include/jemalloc/internal/cache_bin.h b/include/jemalloc/internal/cache_bin.h index 775b71f2..bae669d1 100644 --- a/include/jemalloc/internal/cache_bin.h +++ b/include/jemalloc/internal/cache_bin.h @@ -119,24 +119,23 @@ cache_bin_info_ncached_max(cache_bin_info_t *info) { } static inline cache_bin_sz_t -cache_bin_ncached_get(cache_bin_t *bin, szind_t ind, cache_bin_info_t *infos) { - cache_bin_sz_t n = (cache_bin_sz_t)((infos[ind].stack_size + +cache_bin_ncached_get(cache_bin_t *bin, cache_bin_info_t *info) { + cache_bin_sz_t n = (cache_bin_sz_t)((info->stack_size + bin->full_position - bin->cur_ptr.lowbits) / sizeof(void *)); - assert(n <= cache_bin_info_ncached_max(&infos[ind])); + assert(n <= cache_bin_info_ncached_max(info)); assert(n == 0 || *(bin->cur_ptr.ptr) != NULL); return n; } static inline void ** -cache_bin_empty_position_get(cache_bin_t *bin, szind_t ind, - cache_bin_info_t *infos) { - void **ret = bin->cur_ptr.ptr + cache_bin_ncached_get(bin, ind, infos); +cache_bin_empty_position_get(cache_bin_t *bin, cache_bin_info_t *info) { + void **ret = bin->cur_ptr.ptr + cache_bin_ncached_get(bin, info); /* Low bits overflow disallowed when allocating the space. */ assert((uint32_t)(uintptr_t)ret >= bin->cur_ptr.lowbits); /* Can also be computed via (full_position + ncached_max) | highbits. */ - uintptr_t lowbits = bin->full_position + infos[ind].stack_size; + uintptr_t lowbits = bin->full_position + info->stack_size; uintptr_t highbits = (uintptr_t)bin->cur_ptr.ptr & ~(((uint64_t)1 << 32) - 1); assert(ret == (void **)(lowbits | highbits)); @@ -146,25 +145,24 @@ cache_bin_empty_position_get(cache_bin_t *bin, szind_t ind, /* Returns the numeric value of low water in [0, ncached]. */ static inline cache_bin_sz_t -cache_bin_low_water_get(cache_bin_t *bin, szind_t ind, - cache_bin_info_t *infos) { - cache_bin_sz_t ncached_max = cache_bin_info_ncached_max(&infos[ind]); +cache_bin_low_water_get(cache_bin_t *bin, cache_bin_info_t *info) { + cache_bin_sz_t ncached_max = cache_bin_info_ncached_max(info); cache_bin_sz_t low_water = ncached_max - (cache_bin_sz_t)((bin->low_water_position - bin->full_position) / sizeof(void *)); assert(low_water <= ncached_max); - assert(low_water <= cache_bin_ncached_get(bin, ind, infos)); + assert(low_water <= cache_bin_ncached_get(bin, info)); assert(bin->low_water_position >= bin->cur_ptr.lowbits); return low_water; } static inline void -cache_bin_ncached_set(cache_bin_t *bin, szind_t ind, cache_bin_sz_t n, - cache_bin_info_t *infos) { - bin->cur_ptr.lowbits = bin->full_position + infos[ind].stack_size +cache_bin_ncached_set(cache_bin_t *bin, cache_bin_sz_t n, + cache_bin_info_t *info) { + bin->cur_ptr.lowbits = bin->full_position + info->stack_size - n * sizeof(void *); - assert(n <= cache_bin_info_ncached_max(&infos[ind])); + assert(n <= cache_bin_info_ncached_max(info)); assert(n == 0 || *bin->cur_ptr.ptr != NULL); } @@ -176,11 +174,9 @@ cache_bin_array_descriptor_init(cache_bin_array_descriptor_t *descriptor, descriptor->bins_large = bins_large; } -#define INVALID_SZIND ((szind_t)(unsigned)-1) - JEMALLOC_ALWAYS_INLINE void * -cache_bin_alloc_easy_impl(cache_bin_t *bin, bool *success, szind_t ind, - cache_bin_info_t *infos, const bool adjust_low_water) { +cache_bin_alloc_easy_impl(cache_bin_t *bin, bool *success, + cache_bin_info_t *info, const bool adjust_low_water) { /* * This may read from the empty position; however the loaded value won't * be used. It's safe because the stack has one more slot reserved. @@ -194,9 +190,8 @@ cache_bin_alloc_easy_impl(cache_bin_t *bin, bool *success, szind_t ind, */ if (unlikely(bin->cur_ptr.lowbits > bin->low_water_position)) { if (adjust_low_water) { - assert(ind != INVALID_SZIND); uint32_t empty_position = bin->full_position + - infos[ind].stack_size; + info->stack_size; if (unlikely(bin->cur_ptr.lowbits > empty_position)) { /* Over-allocated; revert. */ bin->cur_ptr.ptr--; @@ -206,7 +201,6 @@ cache_bin_alloc_easy_impl(cache_bin_t *bin, bool *success, szind_t ind, } bin->low_water_position = bin->cur_ptr.lowbits; } else { - assert(ind == INVALID_SZIND); bin->cur_ptr.ptr--; assert(bin->cur_ptr.lowbits == bin->low_water_position); *success = false; @@ -228,19 +222,15 @@ cache_bin_alloc_easy_impl(cache_bin_t *bin, bool *success, szind_t ind, JEMALLOC_ALWAYS_INLINE void * cache_bin_alloc_easy_reduced(cache_bin_t *bin, bool *success) { - /* The szind parameter won't be used. */ - return cache_bin_alloc_easy_impl(bin, success, INVALID_SZIND, - /* infos */ NULL, false); + /* We don't look at info if we're not adjusting low-water. */ + return cache_bin_alloc_easy_impl(bin, success, NULL, false); } JEMALLOC_ALWAYS_INLINE void * -cache_bin_alloc_easy(cache_bin_t *bin, bool *success, szind_t ind, - cache_bin_info_t *infos) { - return cache_bin_alloc_easy_impl(bin, success, ind, infos, true); +cache_bin_alloc_easy(cache_bin_t *bin, bool *success, cache_bin_info_t *info) { + return cache_bin_alloc_easy_impl(bin, success, info, true); } -#undef INVALID_SZIND - JEMALLOC_ALWAYS_INLINE bool cache_bin_dalloc_easy(cache_bin_t *bin, void *ptr) { if (unlikely(bin->cur_ptr.lowbits == bin->full_position)) { @@ -265,17 +255,17 @@ struct cache_bin_ptr_array_s { static inline void cache_bin_ptr_array_init_for_flush(cache_bin_ptr_array_t *arr, cache_bin_t *bin, - cache_bin_sz_t nflush, szind_t ind, cache_bin_info_t *infos) { - arr->ptr = cache_bin_empty_position_get(bin, ind, infos) - 1; - assert(cache_bin_ncached_get(bin, ind, infos) == 0 + cache_bin_sz_t nflush, cache_bin_info_t *info) { + arr->ptr = cache_bin_empty_position_get(bin, info) - 1; + assert(cache_bin_ncached_get(bin, info) == 0 || *arr->ptr != NULL); } static inline void cache_bin_ptr_array_init_for_fill(cache_bin_ptr_array_t *arr, cache_bin_t *bin, - cache_bin_sz_t nfill, szind_t ind, cache_bin_info_t *infos) { - arr->ptr = cache_bin_empty_position_get(bin, ind, infos) - nfill; - assert(cache_bin_ncached_get(bin, ind, infos) == 0); + cache_bin_sz_t nfill, cache_bin_info_t *info) { + arr->ptr = cache_bin_empty_position_get(bin, info) - nfill; + assert(cache_bin_ncached_get(bin, info) == 0); } JEMALLOC_ALWAYS_INLINE void * @@ -290,15 +280,14 @@ cache_bin_ptr_array_set(cache_bin_ptr_array_t *arr, cache_bin_sz_t n, void *p) { static inline void cache_bin_fill_from_ptr_array(cache_bin_t *bin, cache_bin_ptr_array_t *arr, - szind_t ind, szind_t nfilled, cache_bin_info_t *infos) { - assert(cache_bin_ncached_get(bin, ind, infos) == 0); + szind_t nfilled, cache_bin_info_t *info) { + assert(cache_bin_ncached_get(bin, info) == 0); if (nfilled < arr->n) { - void **empty_position = cache_bin_empty_position_get(bin, ind, - infos); + void **empty_position = cache_bin_empty_position_get(bin, info); memmove(empty_position - nfilled, empty_position - arr->n, nfilled * sizeof(void *)); } - cache_bin_ncached_set(bin, ind, nfilled, infos); + cache_bin_ncached_set(bin, nfilled, info); } #endif /* JEMALLOC_INTERNAL_CACHE_BIN_H */ diff --git a/include/jemalloc/internal/tcache_inlines.h b/include/jemalloc/internal/tcache_inlines.h index 28d6e3c8..1b157ba3 100644 --- a/include/jemalloc/internal/tcache_inlines.h +++ b/include/jemalloc/internal/tcache_inlines.h @@ -36,8 +36,8 @@ tcache_alloc_small(tsd_t *tsd, arena_t *arena, tcache_t *tcache, assert(binind < SC_NBINS); bin = tcache_small_bin_get(tcache, binind); - ret = cache_bin_alloc_easy(bin, &tcache_success, binind, - tcache_bin_info); + ret = cache_bin_alloc_easy(bin, &tcache_success, + &tcache_bin_info[binind]); assert(tcache_success == (ret != NULL)); if (unlikely(!tcache_success)) { bool tcache_hard_success; @@ -80,8 +80,8 @@ tcache_alloc_large(tsd_t *tsd, arena_t *arena, tcache_t *tcache, size_t size, assert(binind >= SC_NBINS &&binind < nhbins); bin = tcache_large_bin_get(tcache, binind); - ret = cache_bin_alloc_easy(bin, &tcache_success, binind, - tcache_bin_info); + ret = cache_bin_alloc_easy(bin, &tcache_success, + &tcache_bin_info[binind]); assert(tcache_success == (ret != NULL)); if (unlikely(!tcache_success)) { /* diff --git a/src/arena.c b/src/arena.c index 6b5f1d31..ee357d7f 100644 --- a/src/arena.c +++ b/src/arena.c @@ -200,14 +200,14 @@ arena_stats_merge(tsdn_t *tsdn, arena_t *arena, unsigned *nthreads, for (szind_t i = 0; i < SC_NBINS; i++) { cache_bin_t *tbin = &descriptor->bins_small[i]; arena_stats_accum_zu(&astats->tcache_bytes, - cache_bin_ncached_get(tbin, i, tcache_bin_info) + cache_bin_ncached_get(tbin, &tcache_bin_info[i]) * sz_index2size(i)); } for (szind_t i = 0; i < nhbins - SC_NBINS; i++) { cache_bin_t *tbin = &descriptor->bins_large[i]; arena_stats_accum_zu(&astats->tcache_bytes, - cache_bin_ncached_get(tbin, i + SC_NBINS, - tcache_bin_info) * sz_index2size(i)); + cache_bin_ncached_get(tbin, + &tcache_bin_info[i + SC_NBINS]) * sz_index2size(i)); } } malloc_mutex_prof_read(tsdn, @@ -1321,7 +1321,7 @@ arena_bin_choose_lock(tsdn_t *tsdn, arena_t *arena, szind_t binind, void arena_tcache_fill_small(tsdn_t *tsdn, arena_t *arena, tcache_t *tcache, cache_bin_t *tbin, szind_t binind) { - assert(cache_bin_ncached_get(tbin, binind, tcache_bin_info) == 0); + assert(cache_bin_ncached_get(tbin, &tcache_bin_info[binind]) == 0); tcache->bin_refilled[binind] = true; const bin_info_t *bin_info = &bin_infos[binind]; @@ -1329,8 +1329,8 @@ arena_tcache_fill_small(tsdn_t *tsdn, arena_t *arena, tcache_t *tcache, &tcache_bin_info[binind]) >> tcache->lg_fill_div[binind]; CACHE_BIN_PTR_ARRAY_DECLARE(ptrs, nfill); - cache_bin_ptr_array_init_for_fill(&ptrs, tbin, nfill, binind, - tcache_bin_info); + cache_bin_ptr_array_init_for_fill(&ptrs, tbin, nfill, + &tcache_bin_info[binind]); /* * Bin-local resources are used first: 1) bin->slabcur, and 2) nonfull @@ -1443,8 +1443,8 @@ label_refill: fresh_slab = NULL; } - cache_bin_fill_from_ptr_array(tbin, &ptrs, binind, filled, - tcache_bin_info); + cache_bin_fill_from_ptr_array(tbin, &ptrs, filled, + &tcache_bin_info[binind]); arena_decay_tick(tsdn, arena); } diff --git a/src/tcache.c b/src/tcache.c index 3fc4ee6a..b2d46c3f 100644 --- a/src/tcache.c +++ b/src/tcache.c @@ -59,10 +59,10 @@ tcache_event_hard(tsd_t *tsd, tcache_t *tcache) { is_small = false; } - cache_bin_sz_t low_water = cache_bin_low_water_get(tbin, binind, - tcache_bin_info); - cache_bin_sz_t ncached = cache_bin_ncached_get(tbin, binind, - tcache_bin_info); + cache_bin_sz_t low_water = cache_bin_low_water_get(tbin, + &tcache_bin_info[binind]); + cache_bin_sz_t ncached = cache_bin_ncached_get(tbin, + &tcache_bin_info[binind]); if (low_water > 0) { /* * Flush (ceiling) 3/4 of the objects below the low water mark. @@ -110,8 +110,8 @@ tcache_alloc_small_hard(tsdn_t *tsdn, arena_t *arena, tcache_t *tcache, assert(tcache->arena != NULL); arena_tcache_fill_small(tsdn, arena, tcache, tbin, binind); - ret = cache_bin_alloc_easy(tbin, tcache_success, binind, - tcache_bin_info); + ret = cache_bin_alloc_easy(tbin, tcache_success, + &tcache_bin_info[binind]); return ret; } @@ -168,8 +168,8 @@ tcache_bin_flush_impl(tsd_t *tsd, tcache_t *tcache, cache_bin_t *tbin, } else { assert(binind < nhbins); } - cache_bin_sz_t ncached = cache_bin_ncached_get(tbin, binind, - tcache_bin_info); + cache_bin_sz_t ncached = cache_bin_ncached_get(tbin, + &tcache_bin_info[binind]); assert((cache_bin_sz_t)rem <= ncached); arena_t *tcache_arena = tcache->arena; assert(tcache_arena != NULL); @@ -182,8 +182,8 @@ tcache_bin_flush_impl(tsd_t *tsd, tcache_t *tcache, cache_bin_t *tbin, VARIABLE_ARRAY(edata_t *, item_edata, nflush + 1); CACHE_BIN_PTR_ARRAY_DECLARE(ptrs, nflush); - cache_bin_ptr_array_init_for_flush(&ptrs, tbin, nflush, binind, - tcache_bin_info); + cache_bin_ptr_array_init_for_flush(&ptrs, tbin, nflush, + &tcache_bin_info[binind]); /* Look up edata once per item. */ if (config_opt_safety_checks) { @@ -348,7 +348,7 @@ tcache_bin_flush_impl(tsd_t *tsd, tcache_t *tcache, cache_bin_t *tbin, memmove(tbin->cur_ptr.ptr + (ncached - rem), tbin->cur_ptr.ptr, rem * sizeof(void *)); - cache_bin_ncached_set(tbin, binind, rem, tcache_bin_info); + cache_bin_ncached_set(tbin, rem, &tcache_bin_info[binind]); if (tbin->cur_ptr.lowbits > tbin->low_water_position) { tbin->low_water_position = tbin->cur_ptr.lowbits; } @@ -453,8 +453,8 @@ tcache_bin_init(cache_bin_t *bin, szind_t ind, uintptr_t *stack_cur) { bin->low_water_position = bin->cur_ptr.lowbits; bin->full_position = (uint32_t)(uintptr_t)full_position; assert(bin->cur_ptr.lowbits - bin->full_position == bin_stack_size); - assert(cache_bin_ncached_get(bin, ind, tcache_bin_info) == 0); - assert(cache_bin_empty_position_get(bin, ind, tcache_bin_info) + assert(cache_bin_ncached_get(bin, &tcache_bin_info[ind]) == 0); + assert(cache_bin_empty_position_get(bin, &tcache_bin_info[ind]) == empty_position); return false; @@ -614,8 +614,8 @@ tcache_destroy(tsd_t *tsd, tcache_t *tcache, bool tsd_tcache) { if (tsd_tcache) { /* Release the avail array for the TSD embedded auto tcache. */ cache_bin_t *bin = tcache_small_bin_get(tcache, 0); - assert(cache_bin_ncached_get(bin, 0, tcache_bin_info) == 0); - assert(cache_bin_empty_position_get(bin, 0, tcache_bin_info) == + assert(cache_bin_ncached_get(bin, &tcache_bin_info[0]) == 0); + assert(cache_bin_empty_position_get(bin, &tcache_bin_info[0]) == bin->cur_ptr.ptr); void *avail_array = (void *)((uintptr_t)bin->cur_ptr.ptr - tcache_bin_info[0].stack_size); diff --git a/test/unit/cache_bin.c b/test/unit/cache_bin.c index ab36a3a1..a019ae73 100644 --- a/test/unit/cache_bin.c +++ b/test/unit/cache_bin.c @@ -16,45 +16,45 @@ TEST_BEGIN(test_cache_bin) { bin->cur_ptr.ptr = empty_position; bin->low_water_position = bin->cur_ptr.lowbits; bin->full_position = (uint32_t)(uintptr_t)stack; - expect_ptr_eq(cache_bin_empty_position_get(bin, 0, tcache_bin_info), + expect_ptr_eq(cache_bin_empty_position_get(bin, &tcache_bin_info[0]), empty_position, "Incorrect empty position"); /* Not using expect_zu etc on cache_bin_sz_t since it may change. */ - expect_true(cache_bin_ncached_get(bin, 0, tcache_bin_info) == 0, + expect_true(cache_bin_ncached_get(bin, &tcache_bin_info[0]) == 0, "Incorrect cache size"); bool success; - void *ret = cache_bin_alloc_easy(bin, &success, 0, tcache_bin_info); + void *ret = cache_bin_alloc_easy(bin, &success, &tcache_bin_info[0]); expect_false(success, "Empty cache bin should not alloc"); - expect_true(cache_bin_low_water_get(bin, 0, tcache_bin_info) == 0, + expect_true(cache_bin_low_water_get(bin, &tcache_bin_info[0]) == 0, "Incorrect low water mark"); - cache_bin_ncached_set(bin, 0, 0, tcache_bin_info); + cache_bin_ncached_set(bin, 0, &tcache_bin_info[0]); expect_ptr_eq(bin->cur_ptr.ptr, empty_position, "Bin should be empty"); for (cache_bin_sz_t i = 1; i < ncached_max + 1; i++) { success = cache_bin_dalloc_easy(bin, (void *)(uintptr_t)i); - expect_true(success && cache_bin_ncached_get(bin, 0, - tcache_bin_info) == i, "Bin dalloc failure"); + expect_true(success && cache_bin_ncached_get(bin, + &tcache_bin_info[0]) == i, "Bin dalloc failure"); } success = cache_bin_dalloc_easy(bin, (void *)1); expect_false(success, "Bin should be full"); expect_ptr_eq(bin->cur_ptr.ptr, stack, "Incorrect bin cur_ptr"); - cache_bin_ncached_set(bin, 0, ncached_max, tcache_bin_info); + cache_bin_ncached_set(bin, ncached_max, &tcache_bin_info[0]); expect_ptr_eq(bin->cur_ptr.ptr, stack, "cur_ptr should not change"); /* Emulate low water after refill. */ bin->low_water_position = bin->full_position; for (cache_bin_sz_t i = ncached_max; i > 0; i--) { - ret = cache_bin_alloc_easy(bin, &success, 0, tcache_bin_info); - cache_bin_sz_t ncached = cache_bin_ncached_get(bin, 0, - tcache_bin_info); + ret = cache_bin_alloc_easy(bin, &success, &tcache_bin_info[0]); + cache_bin_sz_t ncached = cache_bin_ncached_get(bin, + &tcache_bin_info[0]); expect_true(success && ncached == i - 1, "Cache bin alloc failure"); expect_ptr_eq(ret, (void *)(uintptr_t)i, "Bin alloc failure"); - expect_true(cache_bin_low_water_get(bin, 0, tcache_bin_info) + expect_true(cache_bin_low_water_get(bin, &tcache_bin_info[0]) == ncached, "Incorrect low water mark"); } - ret = cache_bin_alloc_easy(bin, &success, 0, tcache_bin_info); + ret = cache_bin_alloc_easy(bin, &success, &tcache_bin_info[0]); expect_false(success, "Empty cache bin should not alloc."); expect_ptr_eq(bin->cur_ptr.ptr, stack + ncached_max, "Bin should be empty");