Skip to content

perf: avoid collect#3242

Merged
rakita merged 3 commits intomainfrom
klkvr/avoid-collect
Dec 22, 2025
Merged

perf: avoid collect#3242
rakita merged 3 commits intomainfrom
klkvr/avoid-collect

Conversation

@klkvr
Copy link
Copy Markdown
Collaborator

@klkvr klkvr commented Dec 19, 2025

we can avoid a collect here

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Dec 19, 2025

CodSpeed Performance Report

Merging #3242 will not alter performance

Comparing klkvr/avoid-collect (6107473) with main (da12fae)

Summary

✅ 173 untouched

@klkvr klkvr changed the title perf: avoid collect perf: commit_by_ref Dec 19, 2025
@klkvr klkvr changed the title perf: commit_by_ref perf: avoid collect Dec 19, 2025
@klkvr klkvr force-pushed the klkvr/avoid-collect branch from 5c40743 to cbf000b Compare December 19, 2025 17:36
pub fn apply_evm_state<F>(
&mut self,
evm_state: impl IntoIterator<Item = (Address, Account)>,
pub fn apply_evm_state<'a, F, T>(
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure if this is necessary, but perhaps we should mark this as must_use

Copy link
Copy Markdown
Member

@rakita rakita left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not collecting the Iterator is a footgun that would not apply the change to the state if not collected. So would leave it as is, of course, open to some other solutions.

@klkvr
Copy link
Copy Markdown
Collaborator Author

klkvr commented Dec 22, 2025

moved this to a separate apply_evm_state_iter method that's pub(crate) and is only used by State<DB>

});
if let Some(s) = self.transition_state.as_mut() {
s.add_transitions(transitions)
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need here else { transitions.collect::<Vec<_>>()}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh good catch 6107473

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is why it is a footgun, very hard to catch :)

@rakita rakita merged commit cae8dd3 into main Dec 22, 2025
31 checks passed
@github-actions github-actions bot mentioned this pull request Dec 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants