feat: implement a new type FifoMap and associated tests
#318
Conversation
crates/types/src/types.rs
Outdated
| } | ||
|
|
||
| #[derive(Debug, Clone)] | ||
| pub struct ProfilingMap<K, V, const CAPACITY: usize> { |
There was a problem hiding this comment.
You've made this map generic so that it can accept any type as the key + value (which is cool!!), but you've given it a specific name - ProfilingMap limits this to just store profiles, though the implementation can store anything.
We should either:
- Remove generics and keep the name OR
- Keep generics and make a more generic name like
FifoMap(preferred IMO)
There was a problem hiding this comment.
Additionally, this CAPACITY being passed in as a generic will make it difficult to pass in the capacity at runtime. I think the new function should receive the capacity instead.
BretasArthur1
changed the title
ProfilingMap with FIFO eviction and associated testsFifoMap and associated tests
…re and enabling capacity changes via cli
crates/core/src/surfnet/svm.rs
Outdated
| profile_tag_map: HashMap::new(), | ||
| simulated_transaction_profiles: HashMap::new(), | ||
| executed_transaction_profiles: HashMap::new(), | ||
| simulated_transaction_profiles: FifoMap::default(), // With DEFAULT_PROFILING_MAP_CAPACITY capacity |
There was a problem hiding this comment.
Would it make sense for us to be passing in the CLI param to this new function instead of having a default, then changing it later in initialize?
…pdate related references
…ameter for profiling capacity
lgalabru
left a comment
lgalabru
left a comment
There was a problem hiding this comment.
Looking good, thanks @BretasArthur1 !
Problem
Our currently implementation of holds profile results in a hashmap that keeps growing forever, slowing down the svm more and more.
Solution
Implementing a new type that have a fixed size and holds the elements in the order they enter on the map. Whenever we add a element that should overflow the capacity we pop the first element and add to the last spot the new one(FIFO behavior)