Skip to content

Refactor iter - #108

Merged
OutSquareCapital merged 122 commits into
masterfrom
refactor-iter
Oct 5, 2026
Merged

OutSquareCapital merged 122 commits into
masterfrom
refactor-iter

Conversation

@OutSquareCapital

@OutSquareCapital OutSquareCapital commented Oct 1, 2026 •

Copy link
Copy Markdown
Owner

Main goal

Improve sortedcontainers iterators speed. ALL sortedcontainers call SortedList.__iter__() internally, hence this change will apply to all of them.

for a code equivalent to tuple(iter(SortedList(range(n)))), here are the new numbers with this PR.

size is the nb of items in the Iterator and the number below each library is the median duration in microseconds to complete the task.

size relative
10 2.123
100 1.66
1000 1.442
10000 1.3665
100000 1.344

With reversed iterator:

size relative
10 2.192
100 1.473
1000 1.3555
10000 1.4155
100000 1.279

Other changes

  • a full revamp of benchmark CLI for much more convenient runs and analysis
  • CI checks jobs on github are now much faster with cache use and reduction of redundant work
  • deduplicated a lot of code
  • In general for sortedcontainers, less pointers indirection since we don't need Arc anymore as much
  • AI policy guidelines
  • Architecture documentation

NOTE:
tried to use copilot review. Can catch some useful issues but lot of false positives on polars behavior, and overall greptile is much better. I don't have any money to spend for that left anyway lmao.

impact was almost null, whilst avoiding to codegen 24 different
pyclasses
further reduce code duplication. No significant impact on performance
allowing to directly import Sizes in it, as well as the relevant
test_modules for inspection (otherwise would have to be done by AST)
- use benchmark min instead of median for speed comparison
- change sort order for easier visualisation of last runs
- split `sorted_rs::Iter` in 4 different structs, but no longer generic
- add corresponding pyclasses
- centralise iterators construction with a command enum
- only use Atomics on necessary fields
- code style changes for sorted iterators conversions/constructors
- no need (I THINK) to keep the Arc unused ref in the Inner struct
refactors, only dict views need a smart pointer like this one now.
Swapped it by `Py`
- use derive Deref instead of manual impl
- use `Self` and avoid needless tuple destructuring
OutSquareCapital and others added 21 commits October 5, 2026 01:02
- continue to add iterations test
- harden assertions on `test_remove_below_lower_bound`
- make implicit load value a constant for tests
- narrow the check for `test_remove_below_lower_bound`
double atomic load/store, and two paths only for iterations, with
boolean const to skip checks

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The release panic setting can terminate the Python process on user-triggered errors.

Review effort: Balanced
Findings: 1 High severity

Open (1)
Resolved since last review (5)
Previously missed (2)

In code that hasn't changed since last review

Medium severity Exhausted iterators resume after collection mutation

crates/​sorted_rs/​src/​iter/​core.rs:57

Both forward and reverse null-return paths leave the cursor unchanged, so collection mutations can revive an exhausted iterator. For example, exhaust iter(SortedList([1])) through StopIteration, then add 2: the next call yields 2. Likewise, create a reverse iterator, clear its collection, observe StopIteration, then repopulate it: iteration resumes. Python iterators must stay exhausted. Record a permanent terminal state in both paths, and test mutation after StopIteration; additions before the first exhaustion check should still work.

Medium severity Cache reuses failure index after command changes

scripts/​checks.py:53

The cache stores only a failed index, so changing --slow does not reset it as promised by the CLI help. For example, a fast-mode failure at index 3 is reused by the nine-command slow run because 3 < 9, skipping the earlier checks. Adding or reordering commands can also reuse the wrong index. Store a command-list signature alongside the index and restart at zero when the signature changes.

Comment thread Cargo.toml
@OutSquareCapital
OutSquareCapital merged commit a07ec97 into master Oct 5, 2026
7 checks passed
@OutSquareCapital
OutSquareCapital deleted the refactor-iter branch October 5, 2026 23:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants