Simplified prefetcher api #1988
Conversation
marta-lokhova
force-pushed
the
simplified_prefetcher_api
branch
from
8ea5eb3
to
c9d433d
Compare
src/ledger/LedgerTxn.cpp
Outdated
| res) { | ||
| for (auto const& item : res) | ||
| { | ||
| putInEntryCache(item.first, item.second, LoadType::PREFETCH); |
There was a problem hiding this comment.
Will this overwrite a LoadType::IMMEDIATE with a LoadType::PREFETCH?
(I doubt it's critical either way, just checking if it's intentional either way)
There was a problem hiding this comment.
I think this wouldn't happen since insertIfNotLoaded checks if a key already exists in the cache.
|
Just a couple nits, otherwise looks good! Thanks for taking a more-minimized go at this, it feels much easier to me to understand/predict how it's going to behave (and glad to hear it still gives some perf wins in this form!) |
| @@ -802,7 +804,8 @@ LedgerManagerImpl::closeLedger(LedgerCloseData const& ledgerData) | |||
| // sorted such that sequence numbers are respected | |||
| vector<TransactionFramePtr> txs = ledgerData.getTxSet()->sortForApply(); | |||
|
|
|||
| // first, charge fees | |||
| // first, prefetch source accounts fot txset, then charge fees | |||
| prefetchTxSourceIds(txs); | |||
There was a problem hiding this comment.
What is the benefit of prefetching just the transaction source accounts before prefetching everything else?
There was a problem hiding this comment.
There is no benefit, but it's mostly to accommodate how mOperations in TransactionFrame is populated. Before fees are processed, mOperations is empty (and is populated via a private resetResults method)
marta-lokhova
force-pushed
the
simplified_prefetcher_api
branch
2 times, most recently
from
f0dd4a6
to
c365fb3
Compare
marta-lokhova
force-pushed
the
simplified_prefetcher_api
branch
from
c365fb3
to
09423d7
Compare
|
Updated. |
|
you also need to rebase |
marta-lokhova
force-pushed
the
simplified_prefetcher_api
branch
from
59ac4de
to
9b6f3a9
Compare
|
Updated and rebased (most notable change from the rebase is |
|
r+ 9b6f3a9 |
Simplified prefetcher api Reviewed-by: MonsieurNicolas
|
need to fix |
marta-lokhova
force-pushed
the
simplified_prefetcher_api
branch
from
9b6f3a9
to
72db3e6
Compare
|
This is now updated. |
|
r+ 72db3e6 |
Simplified prefetcher api Reviewed-by: MonsieurNicolas
Resolves #1859
Take 2 at the prefetcher implementation; this one is a much simpler version that avoids any queueing, and simply prefetches a set of unique keys in batches.
I did some preliminary performance testing, and it looks like on postgres, catchup and applying ledger chain for 2000 ledgers took 175 s with prefetching and 230 without. With 5000 ledgers, same test took 548.59 with prefetching, and 650 s without.