Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

Use BufReader and BufWriter - #684

Merged
gavofyork merged 2 commits into
paritytech:masterfrom
tomaka:buffering-file
Sep 7, 2018
Merged

Use BufReader and BufWriter#684
gavofyork merged 2 commits into
paritytech:masterfrom
tomaka:buffering-file

Conversation

@tomaka

Copy link
Copy Markdown
Contributor

Fixes all the performances issues, and should also fix most of the weird disconnects.

@tomakatomaka added the A0-please_review Pull request needs code review. label Sep 7, 2018

let file = fs::File::create(path)?;
serialize(file, &self.store)
serialize(BufWriter::with_capacity(1024 * 1024, file), &self.store)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Curious, how did you get to 1Mb being a good value here?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Considering that the file is 45MB right now, the default of 8kB looked a bit too low to me.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So why 1? Why not 2? Or 5 or 45?

@tomakatomakaSep 7, 2018

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Well, an arbitrary value had to be set.
Ideally we would benchmark multiple values, but this doesn't seem very productive to me at this stage.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

agree with @tomaka , TODO wouldn't hurt though

@dvdplmdvdplm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm

@dvdplm

Copy link
Copy Markdown
Contributor

Build is stumbling on the panic_handler.

@pepyakin

Copy link
Copy Markdown
Contributor

Should be fixed by rebasing to the tip of the master

@gavofyork
gavofyork merged commit 1c8ff0d into paritytech:masterSep 7, 2018
@tomaka
tomaka deleted the buffering-file branch September 7, 2018 17:39
dvdplm added a commit that referenced this pull request Sep 8, 2018
…rs-generic-over-hasher-and-rlpcodec
* origin/master:
Fixed sync stalling when import queue is full (#691)
New extrinsic dispatch model (#678)
remove parachain's Cargo.lock (#682)
Implement json metadata for outer events (#672)
Improvements to the Kademlia system (#688)
Use BufReader and BufWriter (#684)
Switch to using parity/rust:substrate which has rust nightly-2018-08-31 (#686)
Update to latest libp2p (#673)
Implement storage json metadata (#670)
impl MaybeEmpty for H256 and u64 (aka AccountId in prod/tests) (#665)
Speedup compilation (#671)
Remove requirement of function indices for decl_module! (#666)
DigestItem trait (v2) (#650)
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

A0-please_reviewPull request needs code review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@tomaka@dvdplm@pepyakin@gavofyork