refactor(trie): use Vec<(B256, Option<...>)> in HashedPostStateCursors #18868
Conversation
avorylli
requested review from
Rjected,
mediocregopher,
rkrasiuk and
shekhirin
as code owners
There was a problem hiding this comment.
@avorylli This PR is unfortunately not complete, the compare_entries function is not sufficient to merge the in-memory overlay with the DB trie cursor as it does not take into account deleted nodes at all. Please also ensure that CI is passing before marking a PR as ready for review.
Additionally, please make sure you are assigned to an issue before attempting a PR for it, so we don't duplicate work. @quantix9 has said they will be working on an implementation for this, there's no point having competing ones.
| } | ||
|
|
||
| /// Converts hashed post state into [`HashedPostStateSorted`]. | ||
| pub fn into_sorted(self) -> HashedPostStateSorted { |
There was a problem hiding this comment.
You can use drain_into_sorted internally here and deduplicate the implementation. See the TrieUpdates method.
| let mut updated_accounts = Vec::new(); | ||
| let mut destroyed_accounts = HashSet::default(); | ||
| let mut accounts = Vec::new(); | ||
| for (hashed_address, info) in self.accounts.drain() { |
There was a problem hiding this comment.
This for loop could just be let mut accounts = self.accounts.drain().collect::<Vec<_>>() instead.
| let mut non_zero_valued_slots = Vec::new(); | ||
| let mut zero_valued_slots = HashSet::default(); | ||
| let mut storage_slots = Vec::new(); | ||
| for (hashed_slot, value) in self.storage { |
There was a problem hiding this comment.
This for loop could just be let mut storage_slots = self.storage.into_iter().collect::<Vec<_>>() instead.
| /// | ||
| /// This function only checks the post state, not the database, because the latter does not | ||
| /// store destroyed accounts. | ||
| fn is_account_cleared(&self, account: &B256) -> bool { |
There was a problem hiding this comment.
just remove this method
|
|
||
| /// Check if the slot was zeroed out in the post state. | ||
| /// The database is not checked since it already has no zero-valued slots. | ||
| fn is_slot_zero_valued(&self, slot: &B256) -> bool { |
There was a problem hiding this comment.
Just remove this method
CodSpeed Performance ReportMerging #18868 will not alter performanceComparing Summary
|
|
@mediocregopher I completely dont know how to solve these tests, wasted half a day to solve it, feel free to close this pr, and sorry for wasting your time |
2 participants
Summary
Replaces separate
HashSetcollections with unifiedVec<(B256, Option<...>)>pattern inHashedPostStateCursorsfor improved efficiency.Changes
accounts: Vec<(B256, Option<Account>)>whereNoneindicates destroyed accountstorage_slots: Vec<(B256, U256)>whereU256::ZEROindicates deleted slotHashedPostStateAccountCursorandHashedPostStateStorageCursorlogicdestroyed_accounts: B256Setandzero_valued_slots: B256SetCloses: #18848