Use mmap IO for value tables #214
Conversation
arkpar
requested review from
cheme and
MattHalpinParity
and removed request for
cheme
There was a problem hiding this comment.
Lgtm,
I remember having a branch with a trait to be able to choose file or mmap depending on use case as in https://github.com/cheme/parity-db/blob/file_mmap2/src/file.rs.
There was some perf issue, but I was locking write on the mmap, maybe locking read as here, thing would have been good.
I remember bigger issue was grow, not exactly sure why but I ended up with https://github.com/cheme/parity-db/blob/3b49e8724f30573570faab9f8902ad7b65211bc1/src/file.rs#L420 at the time (madvise is missing and mflush is useless).
|
Regarding |
| pub path: std::path::PathBuf, | ||
| pub capacity: AtomicU64, | ||
| pub id: TableId, | ||
| } | ||
|
|
||
| fn map_len(file_len: u64) -> usize { | ||
| file_len as usize + RESERVE_ADDRESS_SPACE |
There was a problem hiding this comment.
Wonder if we also should try to align the size?
There was a problem hiding this comment.
I'm made is so that file size is always aligned.
|
|
||
| // Use different value for tests to work around docker limits on the test machine. | ||
| #[cfg(test)] | ||
| const RESERVE_ADDRESS_SPACE: usize = 64 * 1024 * 1024; // 64 Mb |
There was a problem hiding this comment.
Wondering if this could be configurable (probably these kind of limitation might exist on some cloud infra).
Maybe even a bit more granular (eg more reserved for log).
There was a problem hiding this comment.
We'll give it a round of testing on the parity infrastructure. I've tested on my arm64 vm and had no issues.
| // Leak the old mapping as there might be concurrent readers. | ||
| std::mem::forget(old_map); |
There was a problem hiding this comment.
Why is this leak not an issue? I'm trying to identify the root cause of #233 and this looks like a candidate.
This works much faster when performance is limited by OS page cache. When the database is relatively small and fits entirely in the page cache, making kernel calls becomes too expensive.
Also removed extra copy to a temp buffer when reading values from file.
Also optimized overlay layout a bit. The top level is a vec now instead of a hashmap.