Optimise PVF precompilation before entering active set#5090
Optimise PVF precompilation before entering active set#5090AndreiEres wants to merge 5 commits intomasterfrom
Conversation
AndreiEres
force-pushed
the
AndreiEres/optimize-prepare-pvf
branch
from
d244219 to
78a57ce
Compare
|
The CI pipeline was cancelled due to failure one of the required jobs. |
AndreiEres
added
T0-node
AndreiEres
requested review from
alexggh,
alvicsam,
s0me0ne-unkn0wn and
sandreim
AndreiEres
changed the title
alexggh
left a comment
alexggh
left a comment
There was a problem hiding this comment.
Overall, the changes look correct.
However, I'm not sure if this is even a problem that needs to be fixed, the authority set changes every 4 sessions(4h) on kusama and every 6 sessions(24h) on polkadot, also if we are out of the active set we should have the necessary CPU to run the decompress logic once, the downside is that the state machine is a bit more complicated.
|
|
||
| let Some(session_index) = session_index(sender, relay_parent).await else { return }; | ||
| if state.session_index.map_or(true, |i| session_index > i) { | ||
| state.waiting.clear(); |
There was a problem hiding this comment.
Nit: This if block is big enough, that it can be extracted out in a state.maybe_init_on_new_session().
| // PVF host won't prepare the same code hash twice, so here we just avoid extra communication | ||
| already_prepared_code_hashes: HashSet<ValidationCodeHash>, | ||
| executor_params: Option<ExecutorParams>, | ||
| waiting: HashSet<ValidationCodeHash>, |
There was a problem hiding this comment.
Nit: Let's add some comments what each collection represents.
|
|
||
| /// Checks if artifacts by provided validation code hashes and executor parameters are either | ||
| /// prepared or preparing, and returns the hashes of artifacts that do not meet this criterion. | ||
| pub fn ensure( |
There was a problem hiding this comment.
Nit: I don't think ensure is the right naming for this function since it doesn't makes sure of anything, I would go with something like:
GetUnprepared, ListUnprepared, ArePrepared, to make it clear this is just a query.
| let new_session_index = new_session_index(sender, state.session_index, leaf.hash).await; | ||
| if new_session_index.is_some() { | ||
| state.session_index = new_session_index; | ||
| state.already_prepared_code_hashes.clear(); |
There was a problem hiding this comment.
Wouldn't this be fixed by simply not clearing this or clearing it more infrequent let's say every 24h ?
|
Closed as redundand |
3 participants
Closes #5125
As noticed @sandreim in #4791 (comment), logic of PVF precompilation is not optimal if the node prepares for its following active sets. It would decompress and send PVFs to the Validation Backend it already has.
To solve it, added a new call
ensure_pvfto the ValidationBackend, which gets hashes for not yet prepared PVFs. This way, a session before entering the active set while precompiling PVFs we can avoid wasting CPU time for decompressing already prepared PVFs.Calls to the ValidationBackend are batched. We use three queues:
pendingfor PVFs that definitely need preparation,waitingfor new PVFs andprocessedfor already precompiled. We send the hashes fromwaitingqueue to the ValidationBackend only after we've processed everything in thependingqueue.