Repository navigation
perf: SortedSet.exists and forall walk a set of a reference type through the interface - #46
xperiandri wants to merge 1 commit into
Conversation
…through the interface The code that all reference types share needs a lookup of the exact type at run time for each call of a member of a generic struct, and the struct enumerator of `ImmutableSortedSet<T>` is made of such calls. A walk through `IEnumerable<T>` took 0.53 to 0.58 times as long for strings of 100 to 100 000 elements (13 against 23 ns for an element) and 1.3 to 1.7 times as long for integers, so `typeof<'T>.IsValueType` picks the enumerator. Compiled into the library and measured against a copy of the old loop in the same process, `exists` and `forall` of strings took 0.47 to 0.57 times as long for 100 to 100 000 elements, and the same time for integers (0.94 to 1.03). The cost is the boxed enumerator, 64 bytes per call, which the comment of the functions already counted for `Seq.exists`. The contracts run on integers, so strings get tests of their own: the results of `Set` for the empty set and for the first, a middle, the last and an absent element, and the stop at the first match and at the first failure. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues are identified for the reference-type enumeration optimization.
No open issues 🎉
What changed in this PR
Optimizes SortedSet.exists and SortedSet.forall for reference-type elements while preserving public signatures, results, and short-circuit behavior.
Changes:
- Uses interface enumeration for reference types and retains struct enumeration for value types.
- Updates implementation comments with measured performance and allocation tradeoffs.
- Adds string-based tests for result parity with F#
Setand early termination.
| File | Description |
|---|---|
tests/FSharp.Collections.Immutable.Tests/SortedSet/SortedSetTests.fs |
Adds reference-type search and short-circuit tests. |
src/FSharp.Collections.Immutable/SortedSet/SortedSet.fs |
Selects enumeration by element type and documents performance tradeoffs. |
Review effort: Balanced
df91e9e to
7ab454e
Compare
0459011 to
472d6c6
Compare
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the
|
Proposed Changes
Builds on #35.
SortedSet.existsandSortedSet.forallwalk a set of a reference type throughIEnumerable<T>instead of the struct enumerator ofImmutableSortedSet<T>, and a set of a value type as before. In the code that all reference types share, each call of a member of a generic struct needs a lookup of the exact type at run time, and the struct enumerator ofImmutableSortedSet<T>is made of such calls. No public signature changes.existsandforallof the library against a copy of the old loop, 100 to 100 000 elements, two runsRatios are the time relative to the code before; below 1 is faster. The cost of the interface is the boxed enumerator, 64 bytes per call, which the comment of the functions already counted for
Seq.exists. The base build read 0.94 to 1.08 against the same copy of the old loop, which is the check of the script.The other collections were measured for the same pattern too; only
SortedSetandSortedMapgain (the method, and the tables of every collection and cell, are in the fifth comment of #45: #45 (comment)).The contracts run on integers, so the commit adds tests with strings: the results of
Setfor the empty set and for the first, a middle, the last and an absent element, and the stop at the first match and at the first failure.Types of changes
A performance change that keeps every result and every signature.
Checklist
existsandforallcarries the measured numbers, as the other comments of the module doFurther comments
filterandpartitionalso walk with the struct enumerator, but a builder adds every element they keep. They are unchanged and were not measured.🤖 Generated with Claude Code