Skip to content
New issue

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

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

Already on GitHub? # 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 force-pushed the bez/1947-inter-block-cache branch from 16aa940 to 04c3bac Compare August 15, 2019 16:37
@alexanderbez alexanderbez mentioned this pull request Sep 1, 2019
4 tasks
@@ -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
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Write-through inter-block cache
6 participants