Skip to content

Release consumed itertools.tee buffers - #9322

Merged
youknowone merged 1 commit into
RustPython:mainfrom
1ndahous3:tee_bounded_cache
Oct 23, 2026
Merged

Release consumed itertools.tee buffers#9322
youknowone merged 1 commit into
RustPython:mainfrom
1ndahous3:tee_bounded_cache

Conversation

@1ndahous3

@1ndahous3 1ndahous3 commented Oct 23, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

Found while running more_itertools.triplewise over a stream of objects.

Summary

  • Store tee values in shared blocks and release a block once every copy has advanced past it. Previously, surviving iterators retained the entire input history.
  • Release consumed values outside cursor locks so their finalizers can re-enter iterator code.
  • Check the cache and claim the source under the same lock, preventing a second cursor from advancing the source again for an already-filled slot.

Performance

After consuming 20,000 objects with 1 KiB payloads through more_itertools.triplewise, retained payload falls from 19.5 MiB to 32 KiB on both systems.

Windows 11 x64

Operation Main Patch
Drain two tee copies over 10,000 integers 1.58 ms 1.39 ms
triplewise over 10,000 integers 1.71 ms 1.50 ms
Drain two copies of 10,000 generated tuples 5.88 ms 4.92 ms
Drain the two integer copies consecutively 0.71 ms 0.76 ms

Debian 13 amd64

Operation Main Patch
Drain two tee copies over 10,000 integers 1.35 ms 1.26 ms
triplewise over 10,000 integers 1.49 ms 1.36 ms
Drain two copies of 10,000 generated tuples 5.46 ms 4.57 ms
Drain the two integer copies consecutively 0.64 ms 0.72 ms

Consecutive consumption of the two copies remains slower than main.

AI assistance

Written with Codex (GPT-6), reviewed by a human before submission.

Summary by CodeRabbit

  • Bug Fixes
    • itertools.tee now releases items after all iterator copies have passed them, reducing memory retained during long iterations.
    • Reentrant advancement of a tee iterator now raises a clear runtime error.

@1ndahous3
Release consumed itertools.tee buffers
f6dad81 
Assisted-by: Codex:GPT-6
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 23, 2026 •
edited
Loading

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-23T06:53:05.385855Z f6dad81 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
📥 Commits

Reviewing files that changed from the base of the PR and between e5554e2 and f6dad81.

📒 Files selected for processing: 2
  • crates/vm/src/stdlib/itertools.rs
  • extra_tests/snippets/stdlib_itertools.py

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

itertools.tee now stores cached values in linked blocks of 64. Iterator advancement uses synchronized block references, and consumed blocks can be released as iterators move forward.

Changes: itertools.tee cache

Layer / File(s) Summary
Block storage and iterator setup
crates/vm/src/stdlib/itertools.rs		 PyItertoolsTeeData stores values in linked blocks of 64. Constructors and __copy__ use a mutex-protected block reference.
Block advancement and cache release
crates/vm/src/stdlib/itertools.rs, extra_tests/snippets/stdlib_itertools.py		 next reads cached values or advances the source, moves to a successor block after 64 values, and rejects reentrant advancement or source access. The test checks that an item can be collected after both iterators consume it.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant TeeIterator
  participant TeeDataBlock
  participant SourceIterator
  TeeIterator->>TeeDataBlock: Read cached value or claim source
  TeeDataBlock->>SourceIterator: Advance when cache is empty
  SourceIterator-->>TeeDataBlock: Return next value
  TeeDataBlock-->>TeeIterator: Cache or return value
  TeeIterator->>TeeDataBlock: Move to successor after 64 values
Loading

Suggested reviewers: youknowone

Merge Risk: ⚪ Minimal · up to f6dad

No actionable issue remains in the reviewed tee changes; the PR is mergeable after normal checks.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: releasing consumed buffers held by itertools.tee.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codspeed

codspeed Bot commented Oct 23, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will degrade performance by 12.08%

❌ 1 regressed benchmark
✅ 61 untouched benchmarks
⏩ 4 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
❌ rustpython[comprehension_list.py] 796.9 µs 906.4 µs -12.08%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.

Comparing 1ndahous3:tee_bounded_cache (f6dad81) with main (e5554e2)

Open in CodSpeed

Footnotes

  1. 4 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@youknowone
youknowone merged commit aa3a851 into RustPython:main Oct 23, 2026
21 of 22 checks passed
@1ndahous3
1ndahous3 deleted the tee_bounded_cache branch October 25, 2026 09:34
This was referenced Nov 1, 2026
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@youknowone youknowone youknowone approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@1ndahous3 @youknowone