Refactor Lists to Arrays - #127
Conversation
JonoPrest
commented
Aug 19, 2024
JonoPrest
commented
- Remove use of lists holding eventBatchQueueItem
- Where nodes of lists were returned, replace with a lazy computation on the original orray
a4324d0 to
7a65760
Compare
bba5d06 to
efd937c
Compare
357abaf to
e051c81
Compare
e051c81 to
394eb5b
Compare
|
Busy fixing a few bugs 👍🏼 |
|
Hey @DZakh, the organize utils PR is based on top of this so I won't merge it until this is reviewed so it's a bit easier to separate out the changes. |
| let getFirstArbitraryEventsItem = (queue: array<Types.eventBatchQueueItem>) => | ||
| switch queue[0] { | ||
| | None => None | ||
| | Some(first) => Some((first, () => Array.sliceToEnd(queue, 1))) |
There was a problem hiding this comment.
Note how I've made all these slice operations lazy. So that they only get run if the item on this queue is actually chosen.
3654613 to
5abddb1
Compare
DZakh
left a comment
DZakh
left a comment
There was a problem hiding this comment.
I think ideally for the arbitaryQueue instead of merging everything into a single array with sorting and cloning it on every change we should have:
- An array for each chain (and maybe for every contract registration)
- Metadata which item we consumed for every array
- Metadata which array is consuming and a condition when it should switch to processing another one (we can get it from the smallest eventCmp of the first array item)
- This way it's easier to find an item to process for a specific chain
- Don't need to apply sorting for every item, and only run the check to figure out from which array to get the item next
- Don't need to recreate an array on consuming an item - only increase the pointer index
- When adding more items, just concat them to the array
But it's a bigger refactoring, I don't say we need to do it now. I'll continue reviewing the current changes.
DZakh
left a comment
DZakh
left a comment
There was a problem hiding this comment.
This looks amazing 🚀
Still a little bit painful to look at all the slices and concats, but already a huge improvement!
| ) | ||
| | EventFetchers(item, fetchStatesMap) => (arbitraryEventQueue, fetchStatesMap, item) | ||
| } | ||
| let _ = batch->Js.Array2.push(nextItem) |
There was a problem hiding this comment.
Yeah "batch" is internal to this function.
|
Cool. Lets chat about your points in person, it will be easier 👍🏼 some of them very valid but as you said part of a bigger refactor. |
5abddb1 to
a0428dd
Compare
2 participants
