Prefetch earlier in apply and pipeline it

[mooori]

Changes apply to invoke prefetching of data required for receipts earlier. The aim is to have prefetched data available by the time the corresponding receipt is processed, see #11002.

Current status

Lookahead related bookkeeping is done in PrefetchManager to avoid adding more logic into apply itself. Though the current implementation of PrefetchManager turned out to be a poor abstraction as different kind of receipts (local, delayed, incoming) are handled differently.

Before fixing that I'd like to first measure if prefetching earlier actually yields a performance improvement. If it doesn't, there would be no point in reworking PerformanceManager.

[bowenwang1996]

why don't we do this at the beginning of apply?

[mooori]

The processing order is transactions -> local_receipts -> delayed_receipts where local receipts become available only as transactions are processed. The prefetcher's work queue is FIFO which implies prefetching should happen in the same order as processing. If delayed receipts were prefetched out of order (e.g. at the beginning), it could get in the way of prefetching for local receipts.

Making the prefetcher have a work queue with priorities was brought up, but it's a bigger change.

[codecov]

Codecov Report

Attention: Patch coverage is 90.90909% with 7 lines in your changes are missing coverage. Please review.

Project coverage is 71.03%. Comparing base (6aa1aad) to head (63c0f52).

Files	Patch %	Lines
runtime/runtime/src/lib.rs	90.90%	5 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #11083      +/-   ##
==========================================
+ Coverage   71.02%   71.03%   +0.01%     
==========================================
  Files         773      773              
  Lines      154349   154421      +72     
  Branches   154349   154421      +72     
==========================================
+ Hits       109630   109700      +70     
- Misses      40270    40271       +1     
- Partials     4449     4450       +1     

Flag	Coverage Δ
backward-compatibility	0.24% <0.00%> (-0.01%)	⬇️
db-migration	0.24% <0.00%> (-0.01%)	⬇️
genesis-check	1.42% <0.00%> (-0.01%)	⬇️
integration-tests	36.87% <90.90%> (+<0.01%)	⬆️
linux	69.45% <89.61%> (+0.01%)	⬆️
linux-nightly	70.50% <90.90%> (+0.01%)	⬆️
macos	54.25% <81.81%> (+0.01%)	⬆️
pytests	1.65% <0.00%> (-0.01%)	⬇️
sanity-checks	1.44% <0.00%> (-0.01%)	⬇️
unittests	66.70% <90.90%> (+0.01%)	⬆️
upgradability	0.29% <0.00%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[tayfunelmas]

can this data and logic be just part of the TriePrefetcher instead of having a separate manager on top of it? I see the TriePrefetcher is already aware of receipts and transactions and it can be aware of types of receipts as well (local, delayed, incoming)?

[mooori]

PrefetchManager is scoped to an invocation of fn apply, while TriePrefetcher is scoped to the transaction runtime, which I think is a broader scope. My interpretation is that integrating PrefetchManager functionality into TriePrefetcher would require limiting TriePrefetcher's scope to a particular fn apply invocation as well.

The current PrefetchManager turned out to be a suboptimal abstraction and I'm working on some changes of how delayed receipts are handled. If this results in a performance improvement and the PR is not discarded, I'll consider moving things into TriePrefetcher.