Skip to content

Commit f82ccd9

Browse files
authored
chore: add self-testing code for heap->full_page_size computation (#5772)
Following #5766 I am adding self testing code just to make sure that the patch to mimalloc is correct. We compute full page size like we did before and compare to the fast method. If discrepancy is found we output the error. Signed-off-by: Roman Gershman <[email protected]>
1 parent 24d65c3 commit f82ccd9

File tree

4 files changed

+28
-8
lines changed

4 files changed

+28
-8
lines changed

src/redis/zmalloc.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,10 @@ int zmalloc_get_allocator_wasted_blocks(float ratio, size_t* allocated, size_t*
124124
size_t* wasted);
125125
struct fragmentation_info {
126126
size_t committed;
127+
128+
// a temporary metric to compare against "committed" in production.
129+
// TODO: delete it once we are confident committed is computed correctly.
130+
size_t committed_golden;
127131
size_t wasted;
128132
unsigned bin;
129133
};

src/redis/zmalloc_mi.c

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,9 +202,19 @@ int zmalloc_get_allocator_fragmentation_step(float ratio, struct fragmentation_i
202202

203203
info->bin++;
204204
if (info->bin == MI_BIN_FULL) { // reached end of bins, reset state
205+
info->committed_golden = info->committed;
205206
// Add total comitted size of MI_BIN_FULL that we do not traverse
206207
// as its tracked by zmalloc_heap->full_page_size variable.
207208
info->committed += zmalloc_heap->full_page_size;
209+
210+
// TODO: it's a test code that makes sure `full_page_size` is correct.
211+
// Remove it once we are confident with the implementation.
212+
mi_page_queue_t* pq = &zmalloc_heap->pages[MI_BIN_FULL];
213+
const mi_page_t* page = pq->first;
214+
while (page != NULL) {
215+
info->committed_golden += page->capacity * page->block_size;
216+
page = page->next;
217+
}
208218
info->bin = 0;
209219
return 0;
210220
}

src/server/engine_shard.cc

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,8 @@ bool EngineShard::DefragTaskState::CheckRequired() {
231231
return false;
232232
}
233233

234-
static thread_local fragmentation_info finfo{.committed = 0, .wasted = 0, .bin = 0};
234+
thread_local fragmentation_info finfo{
235+
.committed = 0, .committed_golden = 0, .wasted = 0, .bin = 0};
235236

236237
const std::size_t global_threshold = double(limit) * GetFlag(FLAGS_mem_defrag_threshold);
237238
if (global_threshold > rss_mem_current.load(memory_order_relaxed)) {
@@ -249,19 +250,23 @@ bool EngineShard::DefragTaskState::CheckRequired() {
249250
}
250251

251252
// start checking.
252-
finfo.committed = finfo.wasted = 0;
253+
finfo.committed = finfo.committed_golden = 0;
254+
finfo.wasted = 0;
255+
page_utilization_threshold = GetFlag(FLAGS_mem_defrag_page_utilization_threshold);
253256
}
254257

255258
uint64_t start = absl::GetCurrentTimeNanos();
256-
int res = zmalloc_get_allocator_fragmentation_step(
257-
GetFlag(FLAGS_mem_defrag_page_utilization_threshold), &finfo);
259+
int res = zmalloc_get_allocator_fragmentation_step(page_utilization_threshold, &finfo);
258260
uint64_t duration = absl::GetCurrentTimeNanos() - start;
259-
VLOG_IF(1, duration > 20'000) << "Reading memory usage took " << duration / 1'000
260-
<< " usec on bin " << finfo.bin;
261+
VLOG(1) << "Reading memory usage took " << duration << " ns on bin " << finfo.bin - 1;
262+
261263
if (res == 0) {
262264
// finished checking.
263265
last_check_time = time(nullptr);
264-
266+
if (finfo.committed != finfo.committed_golden) {
267+
LOG_FIRST_N(ERROR, 100) << "committed memory computed incorrectly: " << finfo.committed
268+
<< " vs " << finfo.committed_golden;
269+
}
265270
const double waste_threshold = GetFlag(FLAGS_mem_defrag_waste_threshold);
266271
if (finfo.wasted > size_t(finfo.committed * waste_threshold)) {
267272
VLOG(1) << "memory fragmentation issue found: " << finfo.wasted << " " << finfo.committed;
@@ -359,7 +364,7 @@ uint32_t EngineShard::DefragTask() {
359364
return util::ProactorBase::kOnIdleMaxLevel;
360365
}
361366
}
362-
return 3; // priority.
367+
return 6; // priority.
363368
}
364369

365370
EngineShard::EngineShard(util::ProactorBase* pb, mi_heap_t* heap)

src/server/engine_shard.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,7 @@ class EngineShard {
214214
size_t dbid = 0u;
215215
uint64_t cursor = 0u;
216216
time_t last_check_time = 0;
217+
float page_utilization_threshold = 0.8;
217218

218219
// check the current threshold and return true if
219220
// we need to do the defragmentation

0 commit comments

Comments
 (0)