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

Write-Through Inter-Block Cache #4748

Merged
merged 80 commits into from
Sep 4, 2019
Merged

Conversation

alexanderbez
Copy link
Contributor

@alexanderbez alexanderbez commented Jul 19, 2019

  • Restructure baseapp/ directory by logical grouping to improve readability.
  • Improve BaseApp docs w/ diagrams 🎉
  • Implement CommitKVStoreCacheManager which acts as a persistent inter-block write-through cache.
    • It contains a mapping from StoreKey to a *CommitKVStoreCache (LRU ARC cache).
    • A single instance of CommitKVStoreCacheManager is created in an app and set on the BaseApp.
    • The BaseApp sets this persistent cache on the root CommitMultiStore.
    • When the version is loaded, each IAVL Store is wrapped with it's own respective *CommitKVStoreCache.
    • Writes happen in a write-through fashion.

Preliminary benchmarks show there is only a marginal improvement.

closes: #1947


  • Targeted PR against correct branch (see CONTRIBUTING.md)

  • Linked to github-issue with discussion and accepted design OR link to spec that describes this work.

  • Wrote tests

  • Updated relevant documentation (docs/)

  • Added a relevant changelog entry: clog add [section] [stanza] [message]

  • rereviewed Files changed in the github PR explorer


For Admin Use:

  • Added appropriate labels to PR (ex. wip, ready-for-review, docs)
  • Reviewers Assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

@alexanderbez alexanderbez mentioned this pull request Sep 1, 2019
4 tasks
store/cache/cache.go Outdated Show resolved Hide resolved
@@ -38,13 +37,8 @@ func RandomAccounts(r *rand.Rand, n int) []Account {
// don't need that much entropy for simulation
privkeySeed := make([]byte, 15)
r.Read(privkeySeed)
useSecp := r.Int63()%2 == 0
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this related to the cahce change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not directly, but was needed for initial benchmarking which I henceforth removed. So technically I can undo this change, but we only support secp256k1 keys anyway. Might as well keep it.

alexanderbez and others added 7 commits September 1, 2019 20:52
Co-Authored-By: Aditya <adityasripal@gmail.com>
Co-Authored-By: Aditya <adityasripal@gmail.com>
Co-Authored-By: Aditya <adityasripal@gmail.com>
Co-Authored-By: Aditya <adityasripal@gmail.com>
Copy link
Collaborator

@fedekunze fedekunze left a comment

Choose a reason for hiding this comment

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

utACK

@alexanderbez
Copy link
Contributor Author

@cwgoes I've shared benchmarks and we can see this does indeed offer great improvement, specifically within BeginBlock.

I've addressed all comments @AdityaSripal @tnachen 👍

Copy link
Contributor

@cwgoes cwgoes left a comment

Choose a reason for hiding this comment

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

utACK on the inter-block cache implementation; I didn't review the baseapp changes.

@alexanderbez alexanderbez merged commit f010d2c into master Sep 4, 2019
@alexanderbez alexanderbez deleted the bez/1947-inter-block-cache branch September 4, 2019 17:33
@alexanderbez alexanderbez added this to the v0.38.0 milestone Oct 10, 2019
@alexanderbez alexanderbez mentioned this pull request Oct 10, 2019
4 tasks
larry0x pushed a commit to larry0x/cosmos-sdk that referenced this pull request May 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Write-through inter-block cache
6 participants