feat: add refresh-aware and immutable Sum operators - #1167
Open
giard-alexandre wants to merge 1 commit into
Open
feat: add refresh-aware and immutable Sum operators#1167giard-alexandre wants to merge 1 commit into
giard-alexandre wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
I did a bunch of AI-assisted coding for this, especially when it comes to the tests and benchmark edits as well as creating the comparison tables.
Follow-up for previous PRs that originated from: #1031
Sum operator rewrite
Summary
The
Sumoperator has been rewritten to remove the intermediateForAggregation()andAccumulate()pipeline. The public API now offers two explicit behavior and performance choices:SumSumImmutableThe existing
Sumsignatures are unchanged.SumImmutableadds cache and list overloads forint,long,double,decimal, andfloat, including nullable selectors. TheIAggregateChangeSet<T>overloads remain stateless because refresh information has already been discarded at that point in the pipeline.Implementation changes
Stateful
SumTKey.Observable.Defer, keeping subscriptions isolated from one another.See SumEx.cs.
Stateless
SumImmutableSee SumEx.Immutable.cs.
Tests and API approval
Sum.SumImmutabledoes not re-evaluate refreshed items.See SumFixture.ForCache.cs, SumFixture.ForList.cs, and the API baseline.
Performance results
Methodology
ShortRunjob: one launch, three warmup iterations, and three measured iterations.dda39067. Only its benchmark seed construction was corrected to use isolated snapshots; its operator code was not changed.old time / compared time; values above1.00×are faster than the old implementation and values below1.00×are slower.Aggregate comparison
Sumspeed vs oldSumallocation vs oldSumImmutablespeed vs oldSumImmutableallocation vs oldThe stateful implementation is essentially time-neutral for cache workloads overall, substantially faster for list workloads, and reduces allocation in both.
SumImmutableretains the strongest performance when immutable-item semantics are valid.Cache timing
SumSumImmutableCache allocation
SumSumImmutableList timing
SumSumImmutableList allocation
SumSumImmutableAs you can see, general speedups and reductions in memory allocations across the board. We have a slight slowdown for the new stateful sum operator vs the old implementation but we still get a non-insignificant allocation drop on those runs. I'm open to any recommendations to help improve things any further. I focused mainly on back-compat in this case.