-
Notifications
You must be signed in to change notification settings - Fork 968
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
Some more perf improvements #3204
Conversation
646d225
to
5122be6
Compare
5122be6
to
b7dd10a
Compare
changed "multi order book" to not use |
mChild->forAllWorstBestOffers( | ||
[&](Asset const& buying, Asset const& selling, | ||
std::shared_ptr<OfferDescriptor const>& desc) { | ||
updateWorstBestOffer(AssetPair{buying, selling}, desc); |
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.
I commented earlier about avoiding the copy. I think this one is less scary than the previous one, because OfferDescriptor
s should never leak out of LedgerTxn
. But we should make sure we understand what safety is being sacrificed.
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.
@jonjove I think I addressed your comments
Looks good to me, please squash when you get a chance. @sisuresh I remember you had planned to review this as well, have you done so? |
safe to do: function cannot fail and child is invalidated
remove use of multimap
36d1a07
to
6ac102f
Compare
r+ 6ac102f |
Description
This is a follow up PR from #3180 this time focusing on performance while replaying transactions.
On my laptop the changes yield a 25% speed improvement (real time), broken down as follows:
noexcept
. HistoricallyLedgerTxn
was written in an exception safe way, but we eventually added handlers to just crash in certain scenarios. There are no real benefits of allowing those functions to throw in other situations, as a result we can perform operations that are irreversible. See the next commitInternalLedgerKey
. ~4% speed improvement. We could probably take this idea further, there are other places where we useLedgerKey
as key (that could benefit from this change)mActive
when callingupdateEntry
. 10% speed improvement. In most situations we already know if the key is active, so we can just call the right function.