From 36c6bfb963e8a36a8918eb841902e006466fb7c2 Mon Sep 17 00:00:00 2001 From: David Goldblatt Date: Tue, 18 May 2021 14:52:46 -0700 Subject: [PATCH] SEC: Allow arbitrarily many shards, cached sizes. --- include/jemalloc/internal/pa.h | 4 +-- include/jemalloc/internal/sec.h | 22 ++++----------- src/arena.c | 2 +- src/jemalloc.c | 2 +- src/pa.c | 7 +++-- src/sec.c | 50 +++++++++++++++++++++++---------- test/unit/sec.c | 13 +++++++-- 7 files changed, 59 insertions(+), 41 deletions(-) diff --git a/include/jemalloc/internal/pa.h b/include/jemalloc/internal/pa.h index acb94eb6..cb9f8cff 100644 --- a/include/jemalloc/internal/pa.h +++ b/include/jemalloc/internal/pa.h @@ -130,8 +130,8 @@ bool pa_shard_init(tsdn_t *tsdn, pa_shard_t *shard, emap_t *emap, base_t *base, * This isn't exposed to users; we allow late enablement of the HPA shard so * that we can boot without worrying about the HPA, then turn it on in a0. */ -bool pa_shard_enable_hpa(pa_shard_t *shard, const hpa_shard_opts_t *hpa_opts, - const sec_opts_t *hpa_sec_opts); +bool pa_shard_enable_hpa(tsdn_t *tsdn, pa_shard_t *shard, + const hpa_shard_opts_t *hpa_opts, const sec_opts_t *hpa_sec_opts); /* * We stop using the HPA when custom extent hooks are installed, but still * redirect deallocations to it. diff --git a/include/jemalloc/internal/sec.h b/include/jemalloc/internal/sec.h index ddcdfbdf..fa863382 100644 --- a/include/jemalloc/internal/sec.h +++ b/include/jemalloc/internal/sec.h @@ -13,20 +13,6 @@ * knowledge of the underlying PAI implementation). */ -/* - * This is a *small* extent cache, after all. Assuming 4k pages and an ngroup - * of 4, this allows caching of sizes up to 128k. - */ -#define SEC_NPSIZES 16 -/* - * For now, we put a cap on the number of SECs an arena can have. There's no - * reason it can't be dynamic; it's just inconvenient. This number of shards - * are embedded in the arenas, so there's a space / configurability tradeoff - * here. Eventually, we should probably dynamically allocate only however many - * we require. - */ -#define SEC_NSHARDS_MAX 8 - /* * For now, this is just one field; eventually, we'll probably want to get more * fine-grained data out (like per-size class statistics). @@ -91,7 +77,7 @@ struct sec_shard_s { * hooks are installed. */ bool enabled; - sec_bin_t bins[SEC_NPSIZES]; + sec_bin_t *bins; /* Number of bytes in all bins in the shard. */ size_t bytes_cur; /* The next pszind to flush in the flush-some pathways. */ @@ -104,10 +90,12 @@ struct sec_s { pai_t *fallback; sec_opts_t opts; - sec_shard_t shards[SEC_NSHARDS_MAX]; + sec_shard_t *shards; + pszind_t npsizes; }; -bool sec_init(sec_t *sec, pai_t *fallback, const sec_opts_t *opts); +bool sec_init(tsdn_t *tsdn, sec_t *sec, base_t *base, pai_t *fallback, + const sec_opts_t *opts); void sec_flush(tsdn_t *tsdn, sec_t *sec); void sec_disable(tsdn_t *tsdn, sec_t *sec); diff --git a/src/arena.c b/src/arena.c index 78ea92c1..3ff91572 100644 --- a/src/arena.c +++ b/src/arena.c @@ -1565,7 +1565,7 @@ arena_new(tsdn_t *tsdn, unsigned ind, extent_hooks_t *extent_hooks) { * so arena_hpa_global is not yet initialized. */ if (opt_hpa && ehooks_are_default(base_ehooks_get(base)) && ind != 0) { - if (pa_shard_enable_hpa(&arena->pa_shard, &opt_hpa_opts, + if (pa_shard_enable_hpa(tsdn, &arena->pa_shard, &opt_hpa_opts, &opt_hpa_sec_opts)) { goto label_error; } diff --git a/src/jemalloc.c b/src/jemalloc.c index 613733ff..1f489932 100644 --- a/src/jemalloc.c +++ b/src/jemalloc.c @@ -1781,7 +1781,7 @@ malloc_init_hard_a0_locked() { opt_hpa = false; } } else if (opt_hpa) { - if (pa_shard_enable_hpa(&a0->pa_shard, &opt_hpa_opts, + if (pa_shard_enable_hpa(TSDN_NULL, &a0->pa_shard, &opt_hpa_opts, &opt_hpa_sec_opts)) { return true; } diff --git a/src/pa.c b/src/pa.c index 90809b35..cb3b3df5 100644 --- a/src/pa.c +++ b/src/pa.c @@ -49,13 +49,14 @@ pa_shard_init(tsdn_t *tsdn, pa_shard_t *shard, emap_t *emap, base_t *base, } bool -pa_shard_enable_hpa(pa_shard_t *shard, const hpa_shard_opts_t *hpa_opts, - const sec_opts_t *hpa_sec_opts) { +pa_shard_enable_hpa(tsdn_t *tsdn, pa_shard_t *shard, + const hpa_shard_opts_t *hpa_opts, const sec_opts_t *hpa_sec_opts) { if (hpa_shard_init(&shard->hpa_shard, shard->emap, shard->base, &shard->edata_cache, shard->ind, hpa_opts)) { return true; } - if (sec_init(&shard->hpa_sec, &shard->hpa_shard.pai, hpa_sec_opts)) { + if (sec_init(tsdn, &shard->hpa_sec, shard->base, &shard->hpa_shard.pai, + hpa_sec_opts)) { return true; } shard->ever_used_hpa = true; diff --git a/src/sec.c b/src/sec.c index c37cf35c..41753464 100644 --- a/src/sec.c +++ b/src/sec.c @@ -19,35 +19,55 @@ sec_bin_init(sec_bin_t *bin) { } bool -sec_init(sec_t *sec, pai_t *fallback, const sec_opts_t *opts) { - size_t nshards_clipped = opts->nshards; - if (nshards_clipped > SEC_NSHARDS_MAX) { - nshards_clipped = SEC_NSHARDS_MAX; +sec_init(tsdn_t *tsdn, sec_t *sec, base_t *base, pai_t *fallback, + const sec_opts_t *opts) { + size_t max_alloc = opts->max_alloc & PAGE_MASK; + pszind_t npsizes = sz_psz2ind(max_alloc); + if (sz_pind2sz(npsizes) > opts->max_alloc) { + npsizes--; } - for (size_t i = 0; i < nshards_clipped; i++) { - sec_shard_t *shard = &sec->shards[i]; + size_t sz_shards = opts->nshards * sizeof(sec_shard_t); + size_t sz_bins = opts->nshards * (size_t)npsizes * sizeof(sec_bin_t); + size_t sz_alloc = sz_shards + sz_bins; + void *dynalloc = base_alloc(tsdn, base, sz_alloc, CACHELINE); + if (dynalloc == NULL) { + return true; + } + sec_shard_t *shard_cur = (sec_shard_t *)dynalloc; + sec->shards = shard_cur; + sec_bin_t *bin_cur = (sec_bin_t *)&shard_cur[opts->nshards]; + /* Just for asserts, below. */ + sec_bin_t *bin_start = bin_cur; + + for (size_t i = 0; i < opts->nshards; i++) { + sec_shard_t *shard = shard_cur; + shard_cur++; bool err = malloc_mutex_init(&shard->mtx, "sec_shard", WITNESS_RANK_SEC_SHARD, malloc_mutex_rank_exclusive); if (err) { return true; } shard->enabled = true; - for (pszind_t j = 0; j < SEC_NPSIZES; j++) { + shard->bins = bin_cur; + for (pszind_t j = 0; j < npsizes; j++) { sec_bin_init(&shard->bins[j]); + bin_cur++; } shard->bytes_cur = 0; shard->to_flush_next = 0; } + /* + * Should have exactly matched the bin_start to the first unused byte + * after the shards. + */ + assert((void *)shard_cur == (void *)bin_start); + /* And the last bin to use up the last bytes of the allocation. */ + assert((char *)bin_cur == ((char *)dynalloc + sz_alloc)); sec->fallback = fallback; - size_t max_alloc_clipped = opts->max_alloc; - if (max_alloc_clipped > sz_pind2sz(SEC_NPSIZES - 1)) { - max_alloc_clipped = sz_pind2sz(SEC_NPSIZES - 1); - } sec->opts = *opts; - sec->opts.nshards = nshards_clipped; - sec->opts.max_alloc = max_alloc_clipped; + sec->npsizes = npsizes; /* * Initialize these last so that an improper use of an SEC whose @@ -106,7 +126,7 @@ sec_flush_some_and_unlock(tsdn_t *tsdn, sec_t *sec, sec_shard_t *shard) { /* Update our victim-picking state. */ shard->to_flush_next++; - if (shard->to_flush_next == SEC_NPSIZES) { + if (shard->to_flush_next == sec->npsizes) { shard->to_flush_next = 0; } @@ -249,7 +269,7 @@ sec_flush_all_locked(tsdn_t *tsdn, sec_t *sec, sec_shard_t *shard) { shard->bytes_cur = 0; edata_list_active_t to_flush; edata_list_active_init(&to_flush); - for (pszind_t i = 0; i < SEC_NPSIZES; i++) { + for (pszind_t i = 0; i < sec->npsizes; i++) { sec_bin_t *bin = &shard->bins[i]; bin->bytes_cur = 0; edata_list_active_concat(&to_flush, &bin->freelist); diff --git a/test/unit/sec.c b/test/unit/sec.c index 36ae1a52..01455c89 100644 --- a/test/unit/sec.c +++ b/test/unit/sec.c @@ -37,7 +37,14 @@ test_sec_init(sec_t *sec, pai_t *fallback, size_t nshards, size_t max_alloc, opts.bytes_after_flush = max_bytes / 2; opts.batch_fill_extra = 4; - bool err = sec_init(sec, fallback, &opts); + /* + * We end up leaking this base, but that's fine; this test is + * short-running, and SECs are arena-scoped in reality. + */ + base_t *base = base_new(TSDN_NULL, /* ind */ 123, + &ehooks_default_extent_hooks); + + bool err = sec_init(TSDN_NULL, sec, base, fallback, &opts); assert_false(err, "Unexpected initialization failure"); } @@ -412,10 +419,12 @@ TEST_BEGIN(test_nshards_0) { sec_t sec; /* See the note above -- we can't use the real tsd. */ tsdn_t *tsdn = TSDN_NULL; + base_t *base = base_new(TSDN_NULL, /* ind */ 123, + &ehooks_default_extent_hooks); sec_opts_t opts = SEC_OPTS_DEFAULT; opts.nshards = 0; - sec_init(&sec, &ta.pai, &opts); + sec_init(TSDN_NULL, &sec, base, &ta.pai, &opts); edata_t *edata = pai_alloc(tsdn, &sec.pai, PAGE, PAGE, /* zero */ false);