Mmap index files#69
Conversation
arminsabouri
force-pushed
the
mmap-index-files
branch
3 times, most recently
from
abf88c9 to
fbae67b
Compare
| extra_args+=("--bench" "$FILTER") | ||
| fi | ||
|
|
||
| cargo bench --workspace "${extra_args[@]}" |
There was a problem hiding this comment.
i had a bash array issue when no filter was provided
| cargo bench --workspace "${extra_args[@]}" | |
| if [[ ${#extra_args[@]} -eq 0 ]]; then | |
| cargo bench --workspace | |
| else | |
| cargo bench --workspace "${extra_args[@]}" | |
| fi |
| let len = len_bytes / (N as u64); | ||
| let mmap = if len_bytes > 0 { | ||
| // Safety: the file is complete and will not be written to via this handle. | ||
| Some(unsafe { Mmap::map(&file)? }) |
| } | ||
|
|
||
| fn open(path: impl AsRef<Path>, len_error: &'static str) -> io::Result<Self> { | ||
| let file = OpenOptions::new().read(true).write(true).open(path)?; |
There was a problem hiding this comment.
does it make sense to drop the write(true)?
There was a problem hiding this comment.
I don't think so? Do you see a reason to? We typically write to these files during indexing.
There was a problem hiding this comment.
ah ok, got it
I was concerned about the safety comment, but #69 (comment) is responding it
since mmap is not useful during writing I see no reason to drop it
thank you!
| extra_args+=("--bench" "$FILTER") | ||
| fi | ||
|
|
||
| cargo bench --workspace "${extra_args[@]}" |
There was a problem hiding this comment.
is this bench only timing parse_blocks?
the mmap fast-path kicks in after remap(), so a bench that runs after build_indices would exercise it
There was a problem hiding this comment.
I guess the bench could let build() run in full
There was a problem hiding this comment.
the mmap fast-path kicks in after remap()
Correct. mmap is not useful during writing. buffered writes (something like #71) would be. This is mostly an optimization (perhaps a pre-optimization) for reads during analysis
mmap each of the index files as to avoid repeated disk reads
arminsabouri
force-pushed
the
mmap-index-files
branch
from
fbae67b to
0ac471a
Compare
| cargo bench --workspace | ||
| else | ||
| cargo bench --workspace "${extra_args[@]}" | ||
| fi No newline at end of file |
There was a problem hiding this comment.
is there a formatting problem here?
There was a problem hiding this comment.
No newline at end of file
There was a problem hiding this comment.
Whats the formatting issue?
Repr dense indecies as one struct instead of four individual mut refrences.
arminsabouri
force-pushed
the
mmap-index-files
branch
from
0ac471a to
93a9f4f
Compare
0xZaddyy
left a comment
0xZaddyy
left a comment
There was a problem hiding this comment.
just a little concern which shouldn't prevent this from getting merged, ACK
| let offset = index * (N as u64); | ||
| let offset = (index * N as u64) as usize; | ||
| if let Some(mmap) = &self.mmap { | ||
| let mut buf = [0u8; N]; |
There was a problem hiding this comment.
i was thinking of a bound check here
3 participants
MMap'ing the index files is the main contribution. Also included a refactor to clean up how the indecies were being passed around and built