Reduce BGSAVE/AOF COW by freeing SDS strings in fork child instead of madvise#15260
Reduce BGSAVE/AOF COW by freeing SDS strings in fork child instead of madvise#15260YangboLong wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Reviewed by Cursor Bugbot for commit 02d8ec2. Configure here.
|
i am not sure if this idea can work fine, if there are lots of different size string, |
|
I'm not an expert on memory allocators. But it looks like sdsfree shouldn't trigger COW on the data pages (the entire dismissObject() path is compile-time gated on macro |
|
This change touches performance-sensitive code paths. Adding the |
This PR is based on Valkey PR #905 and the prerequisite
zfree_with_sizemerged via #15071.Replace
dismissMemory(MADV_DONTNEED)withsdsfree()in the fork child's dismiss path for SDS strings.madviseoperates at page granularity and is ineffective for typical string values (< 4KB).sdsfree()lets jemalloc coalesce freed regions and release whole pages back to the OS.object.c:dismissSds()callssdsfree(). Bypass the page-size threshold and THP check forOBJ_STRINGindismissObject()since sdsfree works at any allocation size and doesn't depend on madvise.rdb.c: Lazily-allocated 8KB reusable LZF compression buffer to prevent jemalloc from reusing just-freed pages for compression output.aof.c: Same threshold bypass for the AOF rewrite fork child.Results (3M keys, 80/20 read/write, 10% fragmentation, setup similar to Valkey PR #905):
Without this change, child RSS never decreases during BGSAVE because madvise cannot release sub-page allocations.
Valkey PR #905](valkey-io/valkey#905) reported ~53% COW overhead for 2000B and ~80% for 500B with up to 12M keys.
Note
Medium Risk
Changes fork-child memory release for persistence (strings freed via jemalloc, LZF static buffer); scoped to child with refcount checks, but incorrect free timing could affect rewrite/snapshot correctness.
Overview
This PR reduces copy-on-write (CoW) during BGSAVE and AOF rewrite in the fork child by changing how serialized string data is released after each key is written.
String dismiss path:
dismissSds()now callssdsfree()instead ofdismissMemory(MADV_DONTNEED), so jemalloc can return small string allocations to the OS rather than relying on page-granularmadvise.dismissObject()skips the transparent-huge-pages guard and the half-pagesize_hintcutoff forOBJ_STRING, while non-string types still use the existingmadvise-based dismiss rules.Fork-child call sites:
aof.candrdb.calways invokedismissObject()whenserver.in_fork_child(they no longer require serialized size above half a page before calling in).RDB LZF:
rdbSaveLzfStringObjectuses a lazily allocated 8KB static compression buffer for small outputs so the child does notzmallocinto pages just freed bydismissSds(), which would dirty shared pages and worsen CoW.Reviewed by Cursor Bugbot for commit d8de0fe. Bugbot is set up for automated code reviews on this repo. Configure here.