[Perf] Avoid tx root derivation during block generation #2577
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
vicsn
changed the title
ljedrz
reviewed
Nov 25, 2024
| @@ -105,6 +105,16 @@ impl<N: Network> Deployment<N> { | |||
| Ok(u64::try_from(self.to_bytes_le()?.len())?) | |||
| } | |||
|
|
|||
| /// Returns the number of program functions in the deployment. | |||
| pub fn len(&self) -> usize { | |||
There was a problem hiding this comment.
is len making it obvious that this refers to the number of functions in the program? otherwise num_functions might be a better pick; the same comment applies to is_empty
There was a problem hiding this comment.
Indeed its a bit of an odd naming without context, though it is not unnatural. The Deployment contains just one vector which equals the number of functions/verifying_keys/certificates.
I mainly did this to have consistent naming with Execution::len, where it refers to the number of transitions. This PR then uses both deployment.len() and execution.len() to determine at which "index" to insert an additional Fee transition.
I'd be keen to hear more reviewer's thoughts on this.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
Transaction, deployment and execution root derivation take up 90% of compute of block generation when excluding transaction verification. Fortunately, the ids can be cached at practically 0 cost.
If this is approved, I'll also apply the change to
to_unconfirmed_idand potentially other calls toto_deployment_idandto_execution_id.Test Plan