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

Improved staging shard performance #2034

Merged
merged 2 commits into from
May 4, 2022

Conversation

biospb
Copy link
Contributor

@biospb biospb commented Apr 20, 2022

using map instead of extremely growing array

@@ -51,8 +52,16 @@ func (sa *StagingArea) Commit(dbTx DBTransaction) error {
return errors.New("Attempt to call Commit on already committed stagingArea")
}

for _, shard := range sa.shards {
if shard == nil { // since sa.shards is an array and not a map, some shard slots might be empty.
// order by ID
Copy link
Collaborator

Choose a reason for hiding this comment

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

No need to order


for _, id := range keys {
shard, ok := sa.shards[id]
if !ok || shard == nil {
Copy link
Collaborator

Choose a reason for hiding this comment

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

No need to compare to nil

using map instead of growing array as shard id can be 1000+ in some cases
@biospb biospb force-pushed the Staging-shard-improvement branch from 7de4ede to 2c1e234 Compare May 4, 2022 12:54
someone235
someone235 previously approved these changes May 4, 2022
@codecov
Copy link

codecov bot commented May 4, 2022

Codecov Report

Merging #2034 (48589e2) into dev (2b5202b) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 48589e2 differs from pull request most recent head 4722d1d. Consider uploading reports for the commit 4722d1d to get more accurate results

@@            Coverage Diff             @@
##              dev    #2034      +/-   ##
==========================================
- Coverage   59.42%   59.41%   -0.01%     
==========================================
  Files         675      675              
  Lines       32213    32211       -2     
==========================================
- Hits        19142    19139       -3     
- Misses      10318    10323       +5     
+ Partials     2753     2749       -4     
Impacted Files Coverage Δ
domain/consensus/model/staging_area.go 78.94% <100.00%> (-2.01%) ⬇️
app/protocol/flowcontext/should_mine.go 66.66% <0.00%> (-14.29%) ⬇️
app/protocol/flows/v5/blockrelay/block_locator.go 52.94% <0.00%> (-11.77%) ⬇️
util/mstime/mstime.go 70.00% <0.00%> (-5.00%) ⬇️
domain/miningmanager/mempool/transactions_pool.go 73.77% <0.00%> (-2.46%) ⬇️
infrastructure/network/rpcclient/rpcclient.go 60.67% <0.00%> (-2.25%) ⬇️
.../protocol/flows/v5/blockrelay/handle_relay_invs.go 53.44% <0.00%> (+0.80%) ⬆️
infrastructure/network/addressmanager/store.go 79.11% <0.00%> (+1.26%) ⬆️
infrastructure/network/netadapter/router/route.go 88.57% <0.00%> (+5.71%) ⬆️
...k/netadapter/server/grpcserver/connection_loops.go 65.51% <0.00%> (+8.62%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2b5202b...4722d1d. Read the comment docs.

@someone235 someone235 changed the base branch from master to dev May 4, 2022 15:59
@someone235 someone235 dismissed their stale review May 4, 2022 15:59

The base branch was changed.

@someone235 someone235 merged commit 1660cf0 into kaspanet:dev May 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants