feat: refactor ssload/sstore to JournaledAccount#3145
Conversation
CodSpeed Performance ReportMerging #3145 will improve performances by 25.77%Comparing Summary
Benchmarks breakdown
|
| impl<'a, 'b, ENTRY: JournalEntryTr, DB: Database> JournaledAccount<'a, 'b, ENTRY, DB> { | ||
| /// Creates a new journaled account. | ||
| #[inline(never)] | ||
| pub fn sload( |
There was a problem hiding this comment.
from precompile pov this doesn't provide any additional benefits because in there we can not contrain any types, so we'd need to write additioal dyn compat wrappers for this I believe @klkvr
There was a problem hiding this comment.
wdym by can not contain any types?
Are you talking about this trait? https://github.com/alloy-rs/evm/blob/15975d06bea380a025e59d757367d80d16cacc5a/crates/evm/src/traits.rs#L47
There was a problem hiding this comment.
yeah for EvmInternals we don't propagate any context about DB generic so we'd need to somehow erase it
There was a problem hiding this comment.
Have added JournaledAccountTr that hides generic except for the Error. I see that Error type is fixed so it should be okay.
| 'b, | ||
| <JOURNAL as JournalTr>::JournalEntry, | ||
| <JOURNAL as JournalTr>::Database, |
There was a problem hiding this comment.
if these types are both coming from the JournalTr itself, shouldnt the JournaledAccount then just be generic over that?
There was a problem hiding this comment.
It would be problematic as JournalInner is used in a foundry that does not contain JournalTr
There was a problem hiding this comment.
I can probably drop one lifetime from structure, and restrict it directly in where it is used
rakita
force-pushed
the
rakita/journaled-account-storage
branch
from
f1dbaaf to
9a1585a
Compare
|
Superseeded by #3201 |
3 participants
This will help with perf as condition for account to be loaded will be not needed.
sload/sstore are moved to JournaledAccount for better accesss to it.
This does not solve foundry API problem where they need to directly inject storage into account. Maybe doing
unsafe_sstorewould be a solution, but will leave this for later.