-
Notifications
You must be signed in to change notification settings - Fork 649
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
Improve account maintenance performance #803 #1085
Conversation
libraries/chain/db_balance.cpp
Outdated
@@ -158,6 +160,10 @@ void database::deposit_cashback(const account_object& acct, share_type amount, b | |||
{ | |||
_acct.cashback_vb = *new_vbid; | |||
} ); | |||
modify( acct.statistics( *this ), [&]( account_statistics_object& aso ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will remove the &
here.
1f2c8ef
to
8f08ec9
Compare
if( bal_idx.begin() != bal_idx.end() ) | ||
{ | ||
auto bal_itr = bal_idx.rbegin(); | ||
while( bal_itr->maintenance_flag ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Must check bal_itr != bal_idx.rend(), then you can remove enclosing if.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean change it to while( bal_itr != bal_idx.rend() && bal_itr->maintenance_flag )
? I think my implementation has better performance, since there is only one check outside the loop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, now I see - if bal_idx is not empty() then bal_idx.rbegin() will always exist and the while loop ends when all entries with maintenance_flag have been handled. Ok.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent work @abitmore !
Compared snapshots at block 28M - only difference is the newly added object members.
Thank you @pmconrad. @jmjatlanta: do you want to take a look at this PR before merging? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good and well thought out. Only 1 tiny point, perhaps not worth fixing, but I'll just mention it.
|
||
auto& params = db().get_global_properties().parameters; | ||
const auto& params = global_properties.parameters; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very minor: It looks like this is the only use of global_properties. Perhaps grab params instead of global_properties (line 191)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, nevermind. I see its use further down.
…ormance Improve account maintenance performance bitshares#803
PR for #803.
All existing test cases are passing, not sure if we need additional test cases.
With this patch, on my test machine, time for replaying
27,000,000
blocks (with onlywitness
plugin enabled) reduced from13,812 seconds
to6,112 seconds
(saved55%
).Update: with the 3rd commit (skip the accounts who are not voting), replay time is reduced to
5,633 seconds
(saved59%
overall).Update: successfully synced the chain without issue.