Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: use mmap in NippyJar::load and NippyJarCursor #4634

Merged
merged 1 commit into from
Sep 18, 2023

Conversation

joshieDo
Copy link
Collaborator

PR into #4622

Uses mmap to load the configuration and to read data with the cursor.

@joshieDo joshieDo requested a review from rakita as a code owner September 18, 2023 11:13
@joshieDo joshieDo mentioned this pull request Sep 18, 2023
@joshieDo joshieDo added C-enhancement New feature or request A-bittorrent labels Sep 18, 2023
@codecov
Copy link

codecov bot commented Sep 18, 2023

Codecov Report

Merging #4634 (7552513) into joshie/nippy-offsets (2c064a2) will decrease coverage by 0.03%.
The diff coverage is 90.00%.

Impacted file tree graph

Files Changed Coverage Δ
crates/storage/nippy-jar/src/cursor.rs 80.51% <83.33%> (ø)
crates/storage/nippy-jar/src/lib.rs 95.76% <100.00%> (+0.04%) ⬆️

... and 6 files with indirect coverage changes

Flag Coverage Δ
integration-tests 16.48% <0.00%> (+<0.01%) ⬆️
unit-tests 64.18% <90.00%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
reth binary 30.85% <ø> (ø)
blockchain tree 83.58% <ø> (ø)
pipeline 90.53% <ø> (ø)
storage (db) 75.80% <90.00%> (+<0.01%) ⬆️
trie 94.88% <ø> (ø)
txpool 49.26% <ø> (-0.48%) ⬇️
networking 77.21% <ø> (-0.02%) ⬇️
rpc 57.38% <ø> (+0.01%) ⬆️
consensus 63.40% <ø> (ø)
revm 31.56% <ø> (ø)
payload builder 6.12% <ø> (ø)
primitives 86.51% <ø> (-0.01%) ⬇️

@@ -36,10 +35,13 @@ impl<'a> NippyJarCursor<'a> {
config: &'a NippyJar,
zstd_decompressors: Option<Mutex<Vec<Decompressor<'a>>>>,
) -> Result<Self, NippyJarError> {
let file = File::open(config.data_path())?;
let mmap = unsafe { Mmap::map(&file)? };
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should note that this doesn't makes any guarantees about how/when things are flushed - dirty pages can be flushed at will since we do not do any shadow paging or other sync mechanisms. if that's fine then this PR looks good to me

Copy link
Collaborator Author

@joshieDo joshieDo Sep 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this only deals with read only files/buffer, so it should be fine

@joshieDo joshieDo merged commit 65d02aa into joshie/nippy-offsets Sep 18, 2023
@joshieDo joshieDo deleted the joshie/nippy-mmap branch September 18, 2023 14:45
@shekhirin shekhirin added A-static-files Related to static files and removed A-bittorrent labels Oct 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-static-files Related to static files C-enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants