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

More performance optimizations #1814

Merged
merged 10 commits into from
Aug 3, 2016
Merged

More performance optimizations #1814

merged 10 commits into from
Aug 3, 2016

Conversation

arkpar
Copy link
Collaborator

@arkpar arkpar commented Aug 3, 2016

  • Moved DB commit out of the import lock. DB WAL should not hurt performance anymore on a multicore system
  • Moved RLP compression to background as well
  • Reduced last_hashes cloning
  • Other minor tweaks

@arkpar arkpar added the A0-pleasereview 🤓 Pull request needs code review. label Aug 3, 2016
@coveralls
Copy link

coveralls commented Aug 3, 2016

Coverage Status

Changes Unknown when pulling f723e57 on perf into * on master*.

@coveralls
Copy link

coveralls commented Aug 3, 2016

Coverage Status

Changes Unknown when pulling 9b87657 on perf into * on master*.

@gavofyork
Copy link
Contributor

does the DB write queue form a read cache, also? otherwise how is cohesion enforced while the write-queue is non-empty?

@arkpar
Copy link
Collaborator Author

arkpar commented Aug 3, 2016

It does, that's the whole point

@@ -498,7 +498,7 @@ pub fn enact(
tracing: bool,
db: Box<JournalDB>,
parent: &Header,
last_hashes: LastHashes,
last_hashes: Arc<LastHashes>,
Copy link
Contributor

@rphmeier rphmeier Aug 3, 2016

Choose a reason for hiding this comment

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

seems like all of these Arc<LastHashes> could just be a &LastHashes and we would save a few atomic ops in a bottleneck.

Copy link
Contributor

Choose a reason for hiding this comment

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

and then since LastHashes appears to be just a Vec<H256>, why not just have the last hashes field be a &[H256]?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changing EnvInfo to hold a reference is much more involved. I'll leave it to future PR.

@rphmeier rphmeier added A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. and removed A0-pleasereview 🤓 Pull request needs code review. labels Aug 3, 2016
@rphmeier
Copy link
Contributor

rphmeier commented Aug 3, 2016

a few issues in the unsafe code introduced here and still a little more room for optimization, otherwise LGTM

@arkpar arkpar added A0-pleasereview 🤓 Pull request needs code review. and removed A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. labels Aug 3, 2016
@coveralls
Copy link

coveralls commented Aug 3, 2016

Coverage Status

Coverage increased (+0.02%) to 86.605% when pulling 1613bcc on perf into deceb5f on master.

@gavofyork gavofyork added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Aug 3, 2016
@gavofyork gavofyork merged commit 7093651 into master Aug 3, 2016
@gavofyork gavofyork deleted the perf branch August 3, 2016 20:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants