Add batching to arena bins.

This adds a fast-path for threads freeing a small number of allocations to
bins which are not their "home-base" and which encounter lock contention in
attempting to do so. In producer-consumer workflows, such small lock hold times
can cause lock convoying that greatly increases overall bin mutex contention.

src/arena.c

@@ -661,10 +661,17 @@ arena_bin_slabs_full_remove(arena_t *arena, bin_t *bin, edata_t *slab) {
 }
 
 static void
-arena_bin_reset(tsd_t *tsd, arena_t *arena, bin_t *bin) {
+arena_bin_reset(tsd_t *tsd, arena_t *arena, bin_t *bin, unsigned binind) {
 	edata_t *slab;
 
 	malloc_mutex_lock(tsd_tsdn(tsd), &bin->lock);
+
+	if (arena_bin_has_batch(binind)) {
+		bin_with_batch_t *batched_bin = (bin_with_batch_t *)bin;
+		batcher_init(&batched_bin->remote_frees,
+		    BIN_REMOTE_FREE_ELEMS_MAX);
+	}
+
 	if (bin->slabcur != NULL) {
 		slab = bin->slabcur;
 		bin->slabcur = NULL;
@@ -815,7 +822,8 @@ arena_reset(tsd_t *tsd, arena_t *arena) {
 	/* Bins. */
 	for (unsigned i = 0; i < SC_NBINS; i++) {
 		for (unsigned j = 0; j < bin_infos[i].n_shards; j++) {
-			arena_bin_reset(tsd, arena, arena_get_bin(arena, i, j));
+			arena_bin_reset(tsd, arena, arena_get_bin(arena, i, j),
+			    i);
 		}
 	}
 	pa_shard_reset(tsd_tsdn(tsd), &arena->pa_shard);
@@ -1080,8 +1088,18 @@ arena_cache_bin_fill_small(tsdn_t *tsdn, arena_t *arena,
 	unsigned binshard;
 	bin_t *bin = arena_bin_choose(tsdn, arena, binind, &binshard);
 
+	/*
+	 * This has some fields that are conditionally initialized down batch
+	 * flush pathways.  This can trigger static analysis warnings deeper
+	 * down in the static.  The accesses are guarded by the same checks as
+	 * the initialization, but the analysis isn't able to track that across
+	 * multiple stack frames.
+	 */
+	arena_bin_flush_batch_state_t batch_flush_state
+	    JEMALLOC_CLANG_ANALYZER_SILENCE_INIT({0});
 label_refill:
 	malloc_mutex_lock(tsdn, &bin->lock);
+	arena_bin_flush_batch_after_lock(tsdn, arena, bin, binind, &batch_flush_state);
 
 	while (filled < nfill) {
 		/* Try batch-fill from slabcur first. */
@@ -1136,7 +1154,11 @@ label_refill:
 		cache_bin->tstats.nrequests = 0;
 	}
 
+	arena_bin_flush_batch_before_unlock(tsdn, arena, bin, binind,
+	    &batch_flush_state);
 	malloc_mutex_unlock(tsdn, &bin->lock);
+	arena_bin_flush_batch_after_unlock(tsdn, arena, bin, binind,
+	    &batch_flush_state);
 
 	if (alloc_and_retry) {
 		assert(fresh_slab == NULL);
@@ -1427,12 +1449,16 @@ arena_dalloc_bin(tsdn_t *tsdn, arena_t *arena, edata_t *edata, void *ptr) {
 	malloc_mutex_lock(tsdn, &bin->lock);
 	arena_dalloc_bin_locked_info_t info;
 	arena_dalloc_bin_locked_begin(&info, binind);
-	bool ret = arena_dalloc_bin_locked_step(tsdn, arena, bin,
-	    &info, binind, edata, ptr);
+	edata_t *dalloc_slabs[1];
+	unsigned dalloc_slabs_count = 0;
+	arena_dalloc_bin_locked_step(tsdn, arena, bin, &info, binind, edata,
+	    ptr, dalloc_slabs, /* ndalloc_slabs */ 1, &dalloc_slabs_count,
+	    /* dalloc_slabs_extra */ NULL);
 	arena_dalloc_bin_locked_finish(tsdn, arena, bin, &info);
 	malloc_mutex_unlock(tsdn, &bin->lock);
 
-	if (ret) {
+	if (dalloc_slabs_count != 0) {
+		assert(dalloc_slabs[0] == edata);
 		arena_slab_dalloc(tsdn, arena, edata);
 	}
 }
@@ -1731,7 +1757,7 @@ arena_new(tsdn_t *tsdn, unsigned ind, const arena_config_t *config) {
 	for (unsigned i = 0; i < SC_NBINS; i++) {
 		for (unsigned j = 0; j < bin_infos[i].n_shards; j++) {
 			bin_t *bin = arena_get_bin(arena, i, j);
-			bool err = bin_init(bin);
+			bool err = bin_init(bin, i);
 			if (err) {
 				goto label_error;
 			}

src/bin.c

@@ -6,6 +6,14 @@
 #include "jemalloc/internal/sc.h"
 #include "jemalloc/internal/witness.h"
 
+#ifdef JEMALLOC_JET
+unsigned bin_batching_test_ndalloc_slabs_max = (unsigned)-1;
+void (*bin_batching_test_after_push_hook)(size_t push_idx);
+void (*bin_batching_test_mid_pop_hook)(size_t nelems_to_pop);
+void (*bin_batching_test_after_unlock_hook)(unsigned slab_dalloc_count,
+    bool list_empty);
+#endif
+
 bool
 bin_update_shard_size(unsigned bin_shard_sizes[SC_NBINS], size_t start_size,
     size_t end_size, size_t nshards) {
@@ -39,7 +47,7 @@ bin_shard_sizes_boot(unsigned bin_shard_sizes[SC_NBINS]) {
 }
 
 bool
-bin_init(bin_t *bin) {
+bin_init(bin_t *bin, unsigned binind) {
 	if (malloc_mutex_init(&bin->lock, "bin", WITNESS_RANK_BIN,
 	    malloc_mutex_rank_exclusive)) {
 		return true;
@@ -50,6 +58,11 @@ bin_init(bin_t *bin) {
 	if (config_stats) {
 		memset(&bin->stats, 0, sizeof(bin_stats_t));
 	}
+	if (arena_bin_has_batch(binind)) {
+		bin_with_batch_t *batched_bin = (bin_with_batch_t *)bin;
+		batcher_init(&batched_bin->remote_frees,
+		    opt_bin_info_remote_free_max);
+	}
 	return false;
 }
 
@@ -57,8 +70,23 @@ void
 bin_prefork(tsdn_t *tsdn, bin_t *bin, bool has_batch) {
 	malloc_mutex_prefork(tsdn, &bin->lock);
 	if (has_batch) {
+		/*
+		 * The batch mutex has lower rank than the bin mutex (as it must
+		 * -- it's acquired later).  But during forking, we go
+		 *  bin-at-a-time, so that we acquire mutex on bin 0, then on
+		 *  the bin 0 batcher, then on bin 1.  This is a safe ordering
+		 *  (it's ordered by the index of arenas and bins within those
+		 *  arenas), but will trigger witness errors that would
+		 *  otherwise force another level of arena forking that breaks
+		 *  bin encapsulation (because the witness API doesn't "know"
+		 *  about arena or bin ordering -- it just sees that the batcher
+		 *  has a lower rank than the bin).  So instead we exclude the
+		 *  batcher mutex from witness checking during fork (which is
+		 *  the only time we touch multiple bins at once) by passing
+		 *  TSDN_NULL.
+		 */
 		bin_with_batch_t *batched = (bin_with_batch_t *)bin;
-		batcher_prefork(tsdn, &batched->remote_frees);
+		batcher_prefork(TSDN_NULL, &batched->remote_frees);
 	}
 }
 
@@ -67,7 +95,7 @@ bin_postfork_parent(tsdn_t *tsdn, bin_t *bin, bool has_batch) {
 	malloc_mutex_postfork_parent(tsdn, &bin->lock);
 	if (has_batch) {
 		bin_with_batch_t *batched = (bin_with_batch_t *)bin;
-		batcher_postfork_parent(tsdn, &batched->remote_frees);
+		batcher_postfork_parent(TSDN_NULL, &batched->remote_frees);
 	}
 }
 
@@ -76,6 +104,6 @@ bin_postfork_child(tsdn_t *tsdn, bin_t *bin, bool has_batch) {
 	malloc_mutex_postfork_child(tsdn, &bin->lock);
 	if (has_batch) {
 		bin_with_batch_t *batched = (bin_with_batch_t *)bin;
-		batcher_postfork_child(tsdn, &batched->remote_frees);
+		batcher_postfork_child(TSDN_NULL, &batched->remote_frees);
 	}
 }

src/bin_info.c

@@ -3,8 +3,19 @@
 
 #include "jemalloc/internal/bin_info.h"
 
-size_t opt_bin_info_max_batched_size;
-size_t opt_bin_info_remote_free_max_batch;
+/*
+ * We leave bin-batching disabled by default, with other settings chosen mostly
+ * empirically; across the test programs I looked at they provided the most bang
+ * for the buck.  With other default settings, these choices for bin batching
+ * result in them consuming far less memory (even in the worst case) than the
+ * tcaches themselves, the arena, etc.
+ * Note that we always try to pop all bins on every arena cache bin lock
+ * operation, so the typical memory waste is far less than this (and only on
+ * hot bins, which tend to be large anyways).
+ */
+size_t opt_bin_info_max_batched_size = 0; /* 192 is a good default. */
+size_t opt_bin_info_remote_free_max_batch = 4;
+size_t opt_bin_info_remote_free_max = BIN_REMOTE_FREE_ELEMS_MAX;
 
 bin_info_t bin_infos[SC_NBINS];
 

src/ctl.c

@@ -130,6 +130,7 @@ CTL_PROTO(opt_utrace)
 CTL_PROTO(opt_xmalloc)
 CTL_PROTO(opt_experimental_infallible_new)
 CTL_PROTO(opt_max_batched_size)
+CTL_PROTO(opt_remote_free_max)
 CTL_PROTO(opt_remote_free_max_batch)
 CTL_PROTO(opt_tcache)
 CTL_PROTO(opt_tcache_max)
@@ -483,6 +484,7 @@ static const ctl_named_node_t opt_node[] = {
 	{NAME("experimental_infallible_new"),
 		CTL(opt_experimental_infallible_new)},
 	{NAME("max_batched_size"),	CTL(opt_max_batched_size)},
+	{NAME("remote_free_max"),	CTL(opt_remote_free_max)},
 	{NAME("remote_free_max_batch"),	CTL(opt_remote_free_max_batch)},
 	{NAME("tcache"),	CTL(opt_tcache)},
 	{NAME("tcache_max"),	CTL(opt_tcache_max)},
@@ -2208,6 +2210,8 @@ CTL_RO_NL_CGEN(config_xmalloc, opt_xmalloc, opt_xmalloc, bool)
 CTL_RO_NL_CGEN(config_enable_cxx, opt_experimental_infallible_new,
     opt_experimental_infallible_new, bool)
 CTL_RO_NL_GEN(opt_max_batched_size, opt_bin_info_max_batched_size, size_t)
+CTL_RO_NL_GEN(opt_remote_free_max, opt_bin_info_remote_free_max,
+    size_t)
 CTL_RO_NL_GEN(opt_remote_free_max_batch, opt_bin_info_remote_free_max_batch,
     size_t)
 CTL_RO_NL_GEN(opt_tcache, opt_tcache, bool)

src/jemalloc.c

@@ -1334,6 +1334,11 @@ malloc_conf_init_helper(sc_data_t *sc_data, unsigned bin_shard_sizes[SC_NBINS],
 			    BIN_REMOTE_FREE_ELEMS_MAX,
 			    CONF_DONT_CHECK_MIN, CONF_CHECK_MAX,
 			    /* clip */ true)
+			CONF_HANDLE_SIZE_T(opt_bin_info_remote_free_max,
+			    "remote_free_max", 0,
+			    BIN_REMOTE_FREE_ELEMS_MAX,
+			    CONF_DONT_CHECK_MIN, CONF_CHECK_MAX,
+			    /* clip */ true)
 
 			if (CONF_MATCH("tcache_ncached_max")) {
 				bool err = tcache_bin_info_default_init(

src/stats.c

@@ -1556,6 +1556,7 @@ stats_general_print(emitter_t *emitter) {
 	OPT_WRITE_BOOL("xmalloc")
 	OPT_WRITE_BOOL("experimental_infallible_new")
 	OPT_WRITE_SIZE_T("max_batched_size")
+	OPT_WRITE_SIZE_T("remote_free_max")
 	OPT_WRITE_SIZE_T("remote_free_max_batch")
 	OPT_WRITE_BOOL("tcache")
 	OPT_WRITE_SIZE_T("tcache_max")

src/tcache.c

@@ -325,6 +325,7 @@ tcache_bin_flush_impl_small(tsd_t *tsd, tcache_t *tcache, cache_bin_t *cache_bin
 	assert(binind < SC_NBINS);
 	arena_t *tcache_arena = tcache_slow->arena;
 	assert(tcache_arena != NULL);
+	unsigned tcache_binshard = tsd_binshardsp_get(tsdn_tsd(tsdn))->binshard[binind];
 
 	/*
 	 * Variable length array must have > 0 length; the last element is never
@@ -341,6 +342,18 @@ tcache_bin_flush_impl_small(tsd_t *tsd, tcache_t *tcache, cache_bin_t *cache_bin
 	unsigned dalloc_count = 0;
 	VARIABLE_ARRAY(edata_t *, dalloc_slabs, nflush + 1);
 
+	/*
+	 * There's an edge case where we need to deallocate more slabs than we
+	 * have elements of dalloc_slabs.  This can if we end up deallocating
+	 * items batched by another thread in addition to ones flushed from the
+	 * cache.  Since this is not very likely (most small object
+	 * deallocations don't free up a whole slab), we don't want to burn the
+	 * stack space to keep those excess slabs in an array.  Instead we'll
+	 * maintain an overflow list.
+	 */
+	edata_list_active_t dalloc_slabs_extra;
+	edata_list_active_init(&dalloc_slabs_extra);
+
 	/*
 	 * We're about to grab a bunch of locks.  If one of them happens to be
 	 * the one guarding the arena-level stats counters we flush our
@@ -418,40 +431,136 @@ tcache_bin_flush_impl_small(tsd_t *tsd, tcache_t *tcache, cache_bin_t *cache_bin
 			}
 		}
 
-		/* Actually do the flushing. */
-		malloc_mutex_lock(tsdn, &cur_bin->lock);
 		/*
-		 * Flush stats first, if that was the right lock.  Note that we
-		 * don't actually have to flush stats into the current thread's
-		 * binshard. Flushing into any binshard in the same arena is
-		 * enough; we don't expose stats on per-binshard basis (just
-		 * per-bin).
+		 * We never batch when flushing to our home-base bin shard,
+		 * since it's likely that we'll have to acquire that lock anyway
+		 * when flushing stats.
+		 *
+		 * A plausible check we could add to can_batch is
+		 * '&& arena_is_auto(cur_arena)'.  The motivation would be that
+		 * we have a higher tolerance for dubious user assumptions
+		 * around non-auto arenas (e.g. "if I deallocate every object I
+		 * allocated, and then call tcache.flush, then the arena stats
+		 * must reflect zero live allocations").
+		 *
+		 * This is dubious for a couple reasons:
+		 * - We already don't provide perfect fidelity for stats
+		 *   counting (e.g. for profiled allocations, whose size can
+		 *   inflate in stats).
+		 * - Hanging load-bearing guarantees around stats impedes
+		 *   scalability in general.
+		 *
+		 * There are some "complete" strategies we could do instead:
+		 * - Add a arena.<i>.quiesce call to pop all bins for users who
+		 *   do want those stats accounted for.
+		 * - Make batchability a user-controllable per-arena option.
+		 * - Do a batch pop after every mutex acquisition for which we
+		 *   want to provide accurate stats.  This gives perfectly
+		 *   accurate stats, but can cause weird performance effects
+		 *   (because doing stats collection can now result in slabs
+		 *   becoming empty, and therefore purging, large mutex
+		 *   acquisition, etc.).
+		 * - Propagate the "why" behind a flush down to the level of the
+		 *   batcher, and include a batch pop attempt down full tcache
+		 *   flushing pathways.  This is just a lot of plumbing and
+		 *   internal complexity.
+		 *
+		 * We don't do any of these right now, but the decision calculus
+		 * and tradeoffs are subtle enough that the reasoning was worth
+		 * leaving in this comment.
 		 */
-		if (config_stats && tcache_arena == cur_arena
-		    && !merged_stats) {
-			merged_stats = true;
-			cur_bin->stats.nflushes++;
-			cur_bin->stats.nrequests +=
-			    cache_bin->tstats.nrequests;
-			cache_bin->tstats.nrequests = 0;
+		bool bin_is_batched = arena_bin_has_batch(binind);
+		bool home_binshard = (cur_arena == tcache_arena
+		    && cur_binshard == tcache_binshard);
+		bool can_batch = (flush_start - prev_flush_start
+		    <= opt_bin_info_remote_free_max_batch)
+		    && !home_binshard && bin_is_batched;
+
+		/*
+		 * We try to avoid the batching pathway if we can, so we always
+		 * at least *try* to lock.
+		 */
+		bool locked = false;
+		bool batched = false;
+		if (can_batch) {
+			locked = !malloc_mutex_trylock(tsdn, &cur_bin->lock);
 		}
-		/* Next flush objects. */
-		/* Init only to avoid used-uninitialized warning. */
-		arena_dalloc_bin_locked_info_t dalloc_bin_info = {0};
-		arena_dalloc_bin_locked_begin(&dalloc_bin_info, binind);
-		for (unsigned i = prev_flush_start; i < flush_start; i++) {
-			void *ptr = ptrs->ptr[i];
-			edata_t *edata = item_edata[i].edata;
-			if (arena_dalloc_bin_locked_step(tsdn,
-			    cur_arena, cur_bin, &dalloc_bin_info,
-			    binind, edata, ptr)) {
-				dalloc_slabs[dalloc_count] = edata;
-				dalloc_count++;
+		if (can_batch && !locked) {
+			bin_with_batch_t *batched_bin =
+			    (bin_with_batch_t *)cur_bin;
+			size_t push_idx = batcher_push_begin(tsdn,
+			    &batched_bin->remote_frees,
+			    flush_start - prev_flush_start);
+			bin_batching_test_after_push(push_idx);
+
+			if (push_idx != BATCHER_NO_IDX) {
+				batched = true;
+				unsigned nbatched
+				    = flush_start - prev_flush_start;
+				for (unsigned i = 0; i < nbatched; i++) {
+					unsigned src_ind = prev_flush_start + i;
+					batched_bin->remote_free_data[
+					    push_idx + i].ptr
+						= ptrs->ptr[src_ind];
+					batched_bin->remote_free_data[
+					    push_idx + i].slab
+						= item_edata[src_ind].edata;
+				}
+				batcher_push_end(tsdn,
+				    &batched_bin->remote_frees);
 			}
 		}
-		arena_dalloc_bin_locked_finish(tsdn, cur_arena, cur_bin,
-		    &dalloc_bin_info);
-		malloc_mutex_unlock(tsdn, &cur_bin->lock);
+		if (!batched) {
+			if (!locked) {
+				malloc_mutex_lock(tsdn, &cur_bin->lock);
+			}
+			/*
+			 * Flush stats first, if that was the right lock.  Note
+			 * that we don't actually have to flush stats into the
+			 * current thread's binshard. Flushing into any binshard
+			 * in the same arena is enough; we don't expose stats on
+			 * per-binshard basis (just per-bin).
+			 */
+			if (config_stats && tcache_arena == cur_arena
+			    && !merged_stats) {
+				merged_stats = true;
+				cur_bin->stats.nflushes++;
+				cur_bin->stats.nrequests +=
+				    cache_bin->tstats.nrequests;
+				cache_bin->tstats.nrequests = 0;
+			}
+			unsigned preallocated_slabs = nflush;
+			unsigned ndalloc_slabs = arena_bin_batch_get_ndalloc_slabs(
+			    preallocated_slabs);
+
+			/* Next flush objects our own objects. */
+			/* Init only to avoid used-uninitialized warning. */
+			arena_dalloc_bin_locked_info_t dalloc_bin_info = {0};
+			arena_dalloc_bin_locked_begin(&dalloc_bin_info, binind);
+			for (unsigned i = prev_flush_start; i < flush_start;
+			    i++) {
+				void *ptr = ptrs->ptr[i];
+				edata_t *edata = item_edata[i].edata;
+				arena_dalloc_bin_locked_step(tsdn, cur_arena,
+				    cur_bin, &dalloc_bin_info, binind, edata,
+				    ptr, dalloc_slabs, ndalloc_slabs,
+				    &dalloc_count, &dalloc_slabs_extra);
+			}
+			/*
+			 * Lastly, flush any batched objects (from other
+			 * threads).
+			 */
+			if (bin_is_batched) {
+				arena_bin_flush_batch_impl(tsdn, cur_arena,
+				    cur_bin, &dalloc_bin_info, binind,
+				    dalloc_slabs, ndalloc_slabs,
+				    &dalloc_count, &dalloc_slabs_extra);
+			}
+
+			arena_dalloc_bin_locked_finish(tsdn, cur_arena, cur_bin,
+			    &dalloc_bin_info);
+			malloc_mutex_unlock(tsdn, &cur_bin->lock);
+		}
 		arena_decay_ticks(tsdn, cur_arena,
 		    flush_start - prev_flush_start);
 	}
@@ -460,7 +569,11 @@ tcache_bin_flush_impl_small(tsd_t *tsd, tcache_t *tcache, cache_bin_t *cache_bin
 	for (unsigned i = 0; i < dalloc_count; i++) {
 		edata_t *slab = dalloc_slabs[i];
 		arena_slab_dalloc(tsdn, arena_get_from_edata(slab), slab);
-
+	}
+	while (!edata_list_active_empty(&dalloc_slabs_extra)) {
+		edata_t *slab = edata_list_active_first(&dalloc_slabs_extra);
+		edata_list_active_remove(&dalloc_slabs_extra, slab);
+		arena_slab_dalloc(tsdn, arena_get_from_edata(slab), slab);
 	}
 
 	if (config_stats && !merged_stats) {