feat: Mmap page manager only flushes dirty pages #166
Conversation
🟡 Heimdall Review Status
|
BrianBland
force-pushed
the
brianbland/mmap-flush-range
branch
from
889b11a to
4954405
Compare
src/page/manager/mmap.rs
Outdated
| file_len: AtomicU64, | ||
| page_count: AtomicU32, | ||
| // A set of reallocated dirty pages which need to be flushed to disk. | ||
| dirty_set: Mutex<HashSet<usize>>, |
There was a problem hiding this comment.
nit: should better providing a capacity so that on first few iteration the set doesn't get allocated?
There was a problem hiding this comment.
Possibly, otherwise this is just part of the natural prewarming of the process. The capacity realistically depends on both the size of the database as well as the maximum number of writes per transaction, which makes this difficult to estimate generically.
src/page/manager/mmap.rs
Outdated
| pub fn sync(&self) -> io::Result<()> { | ||
| if cfg!(not(miri)) { | ||
| self.mmap.flush() | ||
| let mut dirty_set = self.dirty_set.lock(); |
There was a problem hiding this comment.
i think this will increase write a lot. Curious about performance number though
src/page/manager/mmap.rs
Outdated
| for offset in dirty_set.drain() { | ||
| self.mmap.flush_range(offset, Page::SIZE)?; | ||
| } | ||
| if let Some(range) = self.dirty_range.lock().take() { |
There was a problem hiding this comment.
Note that, when this is called, the dirty_set is still locked, so this second lock is a bit redundant. I would use a single Mutex around a new DirtyPages struct that holds both the set and the range, but this can be left as an improvement for a future PR.
|
The pages are added to the dirty set when they are requested, and removed from it when Given how or code is structured, this is a purely theoretical problem: |
Tracks dirty pages as a set of non-overlapping non-adjacent runs. Marking a page dirty scales with O(log R) for R runs (worst case O(log P) for P dirty pages), but ensures that only R syscalls need to be made in order to flush to disk. This may be further improved by rounding to discrete chunk sizes, such that R can be divided by a fixed multiple at the cost of some kernel overhead to filter out non-dirty pages.
BrianBland
force-pushed
the
brianbland/mmap-flush-range
branch
from
4954405 to
77a89e6
Compare
cb-heimdall
dismissed
nqd’s stale review
Approved review 3090473464 from nqd is now dismissed due to new commit. Re-request for approval.
This seems to imply that the pages must contain a reference back to their manager in order to perform the cleanup hook. Should we add a pointer back to the |
4 participants
Improves sync performance by only flushing dirty pages instead of scanning the complete file.
Tracks dirty pages as a set of non-overlapping non-adjacent runs. Marking a page dirty scales with
O(log R)forRruns (worst caseO(log P)forPdirty pages), but ensures that onlyRsyscalls need to be made in order to flush to disk.This may be further improved by rounding to discrete chunk sizes, such that
Rcan be divided by a fixed multiple at the cost of some kernel overhead to filter out non-dirty pages.