Skip to content
This repository has been archived by the owner on Jun 9, 2024. It is now read-only.

feat(cachekv): Optimize CacheKV by making SDK Upstream changes. #9

Merged
merged 58 commits into from
Jan 13, 2023

Conversation

itsdevbear
Copy link
Member

No description provided.

itsdevbear and others added 3 commits January 11, 2023 14:13
Co-authored-by: Cal Bera <calbera@berachain.com>
Signed-off-by: Devon Bear <itsdevbear@berachain.com>
Copy link
Member Author

@itsdevbear itsdevbear left a comment

Choose a reason for hiding this comment

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

We can maybe remove the unsorted cache. Lock the tree and then do the tree insert in a gorontinue?

The tree should stay fairly small since it's just a single txn cache (atm) so there is a good chance the overhead of processing the opcode outweighs the tree insert + lock time and we get 0 lock contention and basically free parallelization

}

// Clear the journal entries
store.journalMgr = journal.NewManager()
Copy link
Member Author

Choose a reason for hiding this comment

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

This has to be putting a heavy load on the garbage collected

Copy link
Contributor

Choose a reason for hiding this comment

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

I remember benchmarking this and getting a significant improvement with this line included. Without it, it seems garbage collector had more work to do and it had worse performance @itsdevbear

Copy link
Contributor

Choose a reason for hiding this comment

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

same reason the SortedCache is cleared on line 175

Copy link
Member Author

Choose a reason for hiding this comment

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

@calbera what do you mean again here.

Copy link
Contributor

Choose a reason for hiding this comment

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

basically just including this line store.journalMgr = journal.NewManager() increased benchmark speeds

Copy link
Contributor

Choose a reason for hiding this comment

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

as to the reason why, im not entirely sure, but I'm assuming that we basically help speed up the garbage collector by manually removing all pointers to the those journal CacheEntry structs.

store/cachekv/store.go Outdated Show resolved Hide resolved
store/cachekv/store.go Outdated Show resolved Hide resolved
@itsdevbear itsdevbear changed the base branch from cachekv to main January 13, 2023 02:10
@codecov
Copy link

codecov bot commented Jan 13, 2023

Codecov Report

Merging #9 (74a6573) into main (11bfcd4) will increase coverage by 3.91%.
The diff coverage is 75.82%.

❗ Current head 74a6573 differs from pull request most recent head 5c8679a. Consider uploading reports for the commit 5c8679a to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main       #9      +/-   ##
==========================================
+ Coverage   69.04%   72.95%   +3.91%     
==========================================
  Files          11       17       +6     
  Lines         365      880     +515     
==========================================
+ Hits          252      642     +390     
- Misses        107      228     +121     
- Partials        6       10       +4     
Impacted Files Coverage Δ
core/state/store/journal/manager.go 100.00% <ø> (ø)
core/state/store/cachekv/store.go 61.74% <61.74%> (ø)
core/state/store/cachekv/evm_store.go 72.72% <72.72%> (ø)
core/state/store/cachekv/mergeiterator.go 86.76% <86.76%> (ø)
core/state/store/cachekv/cache_value.go 100.00% <100.00%> (ø)
core/state/store/cachekv/cache_values.go 100.00% <100.00%> (ø)
core/state/store/cachemulti/store.go 100.00% <100.00%> (ø)
lib/ds/trees/btree.go 91.42% <100.00%> (ø)

@itsdevbear itsdevbear added the merge me daddy Trigger Beradozer to bulldoze the PR label Jan 13, 2023
@beradozer beradozer bot merged commit a9d7836 into main Jan 13, 2023
@delete-merged-branch delete-merged-branch bot deleted the upgrade-cachekv branch January 13, 2023 23:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
merge me daddy Trigger Beradozer to bulldoze the PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants