refactor(trie): consolidate StorageTries parallel maps into one

[figtracer]

• created new struct StorageTrieEntry for better cache locality
• updated all calling sites to use this new struct

this makes storage related lookups in the sparse trie 2x faster and we also save 48 bytes per StorageTrie instance

benchmark results:

Op	Size	Old (two maps)	New (single map)	Speedup
Lookup	100	612 ns	280 ns	2.19x
Lookup	1000	7.35 µs	3.31 µs	2.22x
Lookup	5000	35.6 µs	15.6 µs	2.28x
Clear	500	14.2 µs	12.5 µs	1.14x
Insert	5000	1.54 ms	1.51 ms	1.02x

[yongkangc]

Difference:

• Old: Only removes trie, revealed_paths for this account persists
• New: Removes both trie and revealed_paths
  This is a behavior change. If something called take_storage_trie and expected revealed_paths to still be there, it would break.

not 100% sure about this, defer to @mediocregopher or @shekhirin

[mediocregopher]

Good catch, this will indeed break other code. This method is used in two places, and they both do the same thing:

• Call take_storage_trie on an account
• Modify the storage trie
• Put the account's storage trie back using insert_storage_trie

Given this change the first step will discard the revealed_paths, such that on the third step the trie will appear to have no revealed paths.

I think the best thing to do would be to change these methods to take_storage_trie_entry and insert_storage_trie_entry. It will result in a larger PR but I think it's the cleanest solution.

[yongkangc]

Difference in creation logic:

• Old: For a new account, tries to pop from cleared_tries, else clones default_trie. Separately, tries to pop from cleared_revealed_paths, else creates empty HashSet.
• New: Tries to pop from cleared_entries (which has BOTH trie and paths), else creates new entry with cloned default_trie and empty HashSet.

Potential issue: In the old code, cleared_tries and cleared_revealed_paths were independent pools. we could have 10 cleared tries and 5 cleared paths. In the new code, they're always paired in cleared_entries.

[mediocregopher]

Imo this should be fine, I don't believe there was a situation where the length of revealed paths and tries was different in the old code.

[yongkangc]

this is promising, however regarding the trie logic would defer to @mediocregopher or @shekhirin to take a closer look at it

[mattsse]

@shekhirin is this similar to your arena wip?

[mediocregopher]

Imo this should be fine, I don't believe there was a situation where the length of revealed paths and tries was different in the old code.

[mediocregopher]

Good catch, this will indeed break other code. This method is used in two places, and they both do the same thing:

• Call take_storage_trie on an account
• Modify the storage trie
• Put the account's storage trie back using insert_storage_trie

Given this change the first step will discard the revealed_paths, such that on the third step the trie will appear to have no revealed paths.

I think the best thing to do would be to change these methods to take_storage_trie_entry and insert_storage_trie_entry. It will result in a larger PR but I think it's the cleanest solution.