Skip to content

Commit d4e7ffb

Browse files
authored
Improve active defrag in jemalloc 5.2 (redis#9778)
Background: Following the upgrade to jemalloc 5.2, there was a test that used to be flaky and started failing consistently (on 32bit), so we disabled it ​(see redis#9645). This is a test that i introduced in redis#7289 when i attempted to solve a rare stagnation problem, and it later turned out i failed to solve it, ans what's more i added a test that caused it to be not so rare, and as i mentioned, now in jemalloc 5.2 it became consistent on 32bit. Stagnation can happen when all the slabs of the bin are equally utilized, so the decision to move an allocation from a relatively empty slab to a relatively full one, will never happen, and in that test all the slabs are at 50% utilization, so the defragger could just keep scanning the keyspace and not move anything. What this PR changes: * First, finally in jemalloc 5.2 we have the count of non-full slabs, so when we compare the utilization of the current slab, we can compare it to the average utilization of the non-full slabs in our bin, instead of the total average of our bin. this takes the full slabs out of the game, since they're not candidates for migration (neither source nor target). * Secondly, We add some 12% (100/8) to the decision to defrag an allocation, this is the part that aims to avoid stagnation, and it's especially important since the above mentioned change can get us closer to stagnation. * Thirdly, since jemalloc 5.2 adds sharded bins, we take into account all shards (something that's missing from the original PR that merged it), this isn't expected to make any difference since anyway there should be just one shard. How this was benchmarked. What i did was run the memefficiency test unit with `--verbose` and compare the defragger hits and misses the tests reported. At first, when i took into consideration only the non-full slabs, it got a lot worse (i got into stagnation, or just got a lot of misses and a lot of hits), but when i added the 10% i got back to results that were slightly better than the ones of the jemalloc 5.1 branch. i.e. full defragmentation was achieved with fewer hits (relocations), and fewer misses (keyspace scans).
1 parent 362b3b0 commit d4e7ffb

File tree

2 files changed

+23
-16
lines changed

2 files changed

+23
-16
lines changed

deps/jemalloc/include/jemalloc/internal/jemalloc_internal_inlines_c.h

+23-12
Original file line numberDiff line numberDiff line change
@@ -235,23 +235,34 @@ iget_defrag_hint(tsdn_t *tsdn, void* ptr) {
235235
unsigned binshard = extent_binshard_get(slab);
236236
bin_t *bin = &arena->bins[binind].bin_shards[binshard];
237237
malloc_mutex_lock(tsdn, &bin->lock);
238-
/* don't bother moving allocations from the slab currently used for new allocations */
238+
/* Don't bother moving allocations from the slab currently used for new allocations */
239239
if (slab != bin->slabcur) {
240240
int free_in_slab = extent_nfree_get(slab);
241241
if (free_in_slab) {
242242
const bin_info_t *bin_info = &bin_infos[binind];
243-
unsigned long curslabs = bin->stats.curslabs;
244-
size_t curregs = bin->stats.curregs;
245-
if (bin->slabcur) {
246-
/* remove slabcur from the overall utilization */
247-
curregs -= bin_info->nregs - extent_nfree_get(bin->slabcur);
248-
curslabs -= 1;
243+
/* Find number of non-full slabs and the number of regs in them */
244+
unsigned long curslabs = 0;
245+
size_t curregs = 0;
246+
/* Run on all bin shards (usually just one) */
247+
for (uint32_t i=0; i< bin_info->n_shards; i++) {
248+
bin_t *bb = &arena->bins[binind].bin_shards[i];
249+
curslabs += bb->stats.nonfull_slabs;
250+
/* Deduct the regs in full slabs (they're not part of the game) */
251+
unsigned long full_slabs = bb->stats.curslabs - bb->stats.nonfull_slabs;
252+
curregs += bb->stats.curregs - full_slabs * bin_info->nregs;
253+
if (bb->slabcur) {
254+
/* Remove slabcur from the overall utilization (not a candidate to nove from) */
255+
curregs -= bin_info->nregs - extent_nfree_get(bb->slabcur);
256+
curslabs -= 1;
257+
}
249258
}
250-
/* Compare the utilization ratio of the slab in question to the total average,
251-
* to avoid precision lost and division, we do that by extrapolating the usage
252-
* of the slab as if all slabs have the same usage. If this slab is less used
253-
* than the average, we'll prefer to evict the data to hopefully more used ones */
254-
defrag = (bin_info->nregs - free_in_slab) * curslabs <= curregs;
259+
/* Compare the utilization ratio of the slab in question to the total average
260+
* among non-full slabs. To avoid precision loss in division, we do that by
261+
* extrapolating the usage of the slab as if all slabs have the same usage.
262+
* If this slab is less used than the average, we'll prefer to move the data
263+
* to hopefully more used ones. To avoid stagnation when all slabs have the same
264+
* utilization, we give additional 12.5% weight to the decision to defrag. */
265+
defrag = (bin_info->nregs - free_in_slab) * curslabs <= curregs + curregs / 8;
255266
}
256267
}
257268
malloc_mutex_unlock(tsdn, &bin->lock);

tests/unit/memefficiency.tcl

-4
Original file line numberDiff line numberDiff line change
@@ -389,9 +389,6 @@ start_server {tags {"defrag external:skip"} overrides {appendonly yes auto-aof-r
389389
r del biglist1 ;# coverage for quicklistBookmarksClear
390390
} {1}
391391

392-
# Temporarily skip the active defrag edge case since it constantly fails on 32bit bit builds
393-
# since upgrading to jemalloc 5.2.1 (#9623). We need to resolve this and re-enabled.
394-
if {false} {
395392
test "Active defrag edge case" {
396393
# there was an edge case in defrag where all the slabs of a certain bin are exact the same
397394
# % utilization, with the exception of the current slab from which new allocations are made
@@ -494,7 +491,6 @@ start_server {tags {"defrag external:skip"} overrides {appendonly yes auto-aof-r
494491
r save ;# saving an rdb iterates over all the data / pointers
495492
}
496493
}
497-
}
498494
}
499495
}
500496
} ;# run_solo

0 commit comments

Comments
 (0)