You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Default to immediate online mimalloc purging - #367
Deferred mimalloc purging can accumulate reclamation work that later executes on a request worker's allocation path. This changes the online default to immediate purging so eligible memory is reclaimed without the default 60-second delay.
Behavior before and after
Default --mimalloc-purge-delay-ms: 60000 → 0.
Recovery still uses mimalloc's native delay and completes the existing per-worker collection barriers before applying the online setting.
Explicit CLI overrides remain supported, including 60000 to restore the previous policy and -1 to disable purging.
Implementation
Change kDefaultMimallocPurgeDelayMs, which initializes ServerOptions and the CLI's displayed default. Clarify zero in CLI help and document the reclamation tradeoff beside the constant.
Design decisions and alternatives
Immediate purge retains memory reclamation, unlike disabling purge. It shifts work toward frees; it does not make large frees cheap or eliminate ScanHashMap expansion latency.
Prior diagnostic measurements, not tests of this patch:
In the aged-allocation experiment, the previous policy produced later malloc stalls of approximately 81–82 ms. With purge_delay=0, later malloc maxima were 0.274/0.276 ms, while freeing the touched 1 GiB still cost approximately 100 ms in total.
A separate ScanHashMap insertion experiment with purge_delay=0 still observed approximately 10 ms when retiring a 128 MiB old table, including approximately 9.5 ms in MADV_DONTNEED. These are different experiments and must not be interpreted as an 82 ms → 10 ms map A/B comparison.
The existing SPDK server comparison used PR #366 at db2b5a5d42a3cbc0980b4c97b8905d94e07a940e: retain 1 billion keys, 80 connections issuing random overwrite SETs, 16 threads, 1024-byte values, no key prefix, and defrag block sleep 100 ms. Both data nodes were restarted for each arm; each arm had 120 seconds of warmup and 600 seconds measured using merged complete 5-second HDR windows.
Online purge delay
SET/s
Pooled p99.99 (ms)
Worst 5-second p99.99 (ms)
Max (ms)
Previous default, first run
254,854
0.874
3.625
5.451
0 ms
229,185
0.890
3.857
6.835
Previous default, repeat
221,801
0.925
3.421
5.675
The first run had no active primary defrag; both subsequent runs did, with different reclamation intensity. The measurements show no clear overall tail improvement or regression, and do not establish performance equivalence or isolate throughput effects. This overwrite workload does not exercise map doubling. All measured windows were complete with no HDR drops/overflow, connection errors were zero, and both nodes retained exactly 1 billion keys.
Documentation and comments
Updated the nearby policy comment and CLI help. No architectural lifecycle, module boundary, or recovery invariant changes.
Test plan
pre-commit run clang-format --files include/lavik/server.h app/lavik.cpp: passed with clang-format 23.1.1.
git diff --cached --check: passed.
pre-commit run --files include/lavik/server.h app/lavik.cpp: blocked while initializing the unrelated Prettier hook by npm EALLOWGIT; the applicable C++ hook passed separately.
Full Linux build/runtime tests were not run locally on macOS; PR CI supplies those checks. The measurements above exercised the explicit setting on earlier builds, not this new commit.
Risk assessment
Large or batched frees can synchronously purge and still stall a worker. Immediate reclamation can also increase later page-fault/recommit work. This is a default tuning change, not a complete fix for long-tail latency.
Rollback plan
Set --mimalloc-purge-delay-ms=60000 or revert this commit.
Reviewer guide
Review the default and tradeoff comment in include/lavik/server.h, then the help text in app/lavik.cpp. The post-recovery option application in src/redis/server.cpp is unchanged.
Follow-up work
Reducing the remaining synchronous cost of retiring large ScanHashMap tables requires a separate change.
Summary by CodeRabbit
Changed
The default memory purge delay is now 0 milliseconds, so eligible pages are purged immediately during normal operation.
During recovery, mimalloc’s native purge delay remains in effect until workers finish.
Updated command-line help clarifies that the online purge delay applies after recovery; 0 purges immediately, while -1 disables purging.
No actionable comments were generated in the recent review. 🎉
ℹ️ Recent review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: 873ad61d-2c0f-4381-8be3-dc833eeb6e91
📥 Commits
Reviewing files that changed from the base of the PR and between 91f0c89 and 0c481a2.
📒 Files selected for processing: 2
app/lavik.cpp
include/lavik/server.h
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.
Walkthrough
The default mimalloc purge delay changes from 60 seconds to 0 milliseconds. The command-line help explains that the setting applies after recovery, that 0 purges immediately, and that -1 disables purging.
Changes
Mimalloc purge delay
Layer / File(s)
Summary
Default and command-line help include/lavik/server.h, app/lavik.cpp
The default changes to 0 milliseconds. Comments describe immediate online purging, possible synchronous costs for large frees, and delayed purging during recovery. The command-line help documents when the setting applies and the meanings of 0 and -1.
The change is mergeable with normal CI validation; no actionable defect was found in the reviewed scope.
Pre-merge checks: 4 · 1
Failed checks: 1 warning
Check name
Status
Explanation
Resolution
Docstring Coverage
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files.
Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed checks: 4 passed
Check name
Status
Explanation
Title check
The title clearly and concisely describes the primary change: making online mimalloc purging immediate by default.
Description check
The description covers all required sections, explains the behavior change and design tradeoffs, documents comments, lists validation commands and results, and includes risk, rollback, reviewer, and f…
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Full details: Description check
Explanation
The description covers all required sections, explains the behavior change and design tradeoffs, documents comments, lists validation commands and results, and includes risk, rollback, reviewer, and follow-up information.
Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
A rabbit checks the purge delay, Zero now clears pages right away. After recovery, workers run, Then purging starts when work is done. Large frees may pause the rabbit’s feet, While minus one leaves purging asleep.
Comment @coderabbitai help to get the list of available commands.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
Deferred mimalloc purging can accumulate reclamation work that later executes on a request worker's allocation path. This changes the online default to immediate purging so eligible memory is reclaimed without the default 60-second delay.
Behavior before and after
--mimalloc-purge-delay-ms:60000→0.60000to restore the previous policy and-1to disable purging.Implementation
Change
kDefaultMimallocPurgeDelayMs, which initializesServerOptionsand the CLI's displayed default. Clarify zero in CLI help and document the reclamation tradeoff beside the constant.Design decisions and alternatives
Immediate purge retains memory reclamation, unlike disabling purge. It shifts work toward frees; it does not make large frees cheap or eliminate ScanHashMap expansion latency.
Prior diagnostic measurements, not tests of this patch:
purge_delay=0, later malloc maxima were 0.274/0.276 ms, while freeing the touched 1 GiB still cost approximately 100 ms in total.purge_delay=0still observed approximately 10 ms when retiring a 128 MiB old table, including approximately 9.5 ms inMADV_DONTNEED. These are different experiments and must not be interpreted as an 82 ms → 10 ms map A/B comparison.The existing SPDK server comparison used PR #366 at
db2b5a5d42a3cbc0980b4c97b8905d94e07a940e: retain 1 billion keys, 80 connections issuing random overwrite SETs, 16 threads, 1024-byte values, no key prefix, and defrag block sleep 100 ms. Both data nodes were restarted for each arm; each arm had 120 seconds of warmup and 600 seconds measured using merged complete 5-second HDR windows.The first run had no active primary defrag; both subsequent runs did, with different reclamation intensity. The measurements show no clear overall tail improvement or regression, and do not establish performance equivalence or isolate throughput effects. This overwrite workload does not exercise map doubling. All measured windows were complete with no HDR drops/overflow, connection errors were zero, and both nodes retained exactly 1 billion keys.
Documentation and comments
Updated the nearby policy comment and CLI help. No architectural lifecycle, module boundary, or recovery invariant changes.
Test plan
pre-commit run clang-format --files include/lavik/server.h app/lavik.cpp: passed with clang-format 23.1.1.git diff --cached --check: passed.pre-commit run --files include/lavik/server.h app/lavik.cpp: blocked while initializing the unrelated Prettier hook by npmEALLOWGIT; the applicable C++ hook passed separately.Risk assessment
Large or batched frees can synchronously purge and still stall a worker. Immediate reclamation can also increase later page-fault/recommit work. This is a default tuning change, not a complete fix for long-tail latency.
Rollback plan
Set
--mimalloc-purge-delay-ms=60000or revert this commit.Reviewer guide
Review the default and tradeoff comment in
include/lavik/server.h, then the help text inapp/lavik.cpp. The post-recovery option application insrc/redis/server.cppis unchanged.Follow-up work
Reducing the remaining synchronous cost of retiring large ScanHashMap tables requires a separate change.
Summary by CodeRabbit
0purges immediately, while-1disables purging.