Use of generators to reduce memory usage via a LazyMultiSet#82
Use of generators to reduce memory usage via a LazyMultiSet#82samwillis wants to merge 19 commits intoelectric-sql:mainfrom
Conversation
…sing Co-authored-by: sam.willis <[email protected]>
…ility Co-authored-by: sam.willis <[email protected]>
Co-authored-by: sam.willis <[email protected]>
Co-authored-by: sam.willis <[email protected]>
Co-authored-by: sam.willis <[email protected]>
Co-authored-by: sam.willis <[email protected]>
Co-authored-by: sam.willis <[email protected]>
Co-authored-by: sam.willis <[email protected]>
Co-authored-by: sam.willis <[email protected]>
Co-authored-by: sam.willis <[email protected]>
…ance Co-authored-by: sam.willis <[email protected]>
Co-authored-by: sam.willis <[email protected]>
Co-authored-by: sam.willis <[email protected]>
Co-authored-by: sam.willis <[email protected]>
Co-authored-by: sam.willis <[email protected]>
Co-authored-by: sam.willis <[email protected]>
Co-authored-by: sam.willis <[email protected]>
Co-authored-by: sam.willis <[email protected]>
samwillis
changed the title
kevin-dp
left a comment
kevin-dp
left a comment
There was a problem hiding this comment.
LGTM implementation-wise. However, i do think we should carefully benchmark performance and memory usage before merging as it does complicate the code quite a bit so i want to be 100% sure this is the way to go.
| return new MultiSet(result) | ||
| } | ||
|
|
||
| *lazyJoin<V2>(other: Index<K, V2>): Generator<[[K, [V, V2]], number], void, unknown> { |
There was a problem hiding this comment.
Let's remove the duplication between both branches like i did here: ac24ff0
| const consolidated = new MultiSet(values).consolidate() | ||
| const sortedValues = consolidated | ||
| .getInner() | ||
| const consolidated = LazyMultiSet.fromArray(values).consolidate() |
There was a problem hiding this comment.
Why creating a lazy multiset to then consolidate it (which materializes it) to then immediately copy it into an array. Seems like a lot of overhead for something we could do by reducing once over the original array (i.e. loop over values and consolidate it into an array of consolidated values).
| }) | ||
| }) | ||
|
|
||
| it('should consolidate correctly', () => { |
There was a problem hiding this comment.
Let's also check the corner case where the multiplicities for a certain value cancel out and that the result does not contain the value.
Absolutely agree, stack traces and and performance traces of individual operators will likely get more complex. the key things I want is to check:
The latter will be hard to measure as it not guarantied to happen during execution. We need to do manual testing with performance traces and inspect what happens. Likely need to do this in the browser. Peak mem is a good indication of what GC needs to be done relative to the two versions. |
3 participants
Also converts almost all operators to use them wherever possible.
We should benchmark this against main to see how what it does with the memory usage and allocations.