Avoid redundant work in set difference operations by 1ndahous3 · Pull Request #8869 · RustPython/RustPython

1ndahous3 · 2026-09-27T20:06:01Z

This PR follows our AI policy.

Summary

Copy set and frozenset difference results once, including multi-argument differences.
Reuse stored hashes after deduplicating generic symmetric-difference inputs, avoiding repeated __hash__ calls.
Consume difference_update iterables incrementally, avoiding a temporary list of all elements and retaining completed removals when iteration raises.

Continues the stored-hash work in #8852 and #8859 for generic symmetric-difference inputs.

Performance

Release build, Windows x64; medians of seven samples against the same main revision.

Operation	Before	After	Speedup
Difference: 4,096 integers, one 100-item exclusion set	67.2 µs	33.6 µs	2.00×
Difference: 4,096 integers, two 100-item exclusion sets	97.9 µs	35.1 µs	2.79×
Symmetric difference: list of 512 keys with Python `__hash__`	572.2 µs	382.3 µs	1.50×

AI assistance

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

Summary by CodeRabbit

Bug Fixes
- Set and frozenset difference operations now correctly handle multiple operands and reflected subtraction without changing the original operands.
- Symmetric difference operations now handle duplicate inputs consistently, including when updating a set in place.
- difference_update now applies removals as items are yielded, so changes made before an iterator raises an error are retained. Updating a set from itself also correctly empties it.

Assisted-by: Codex:GPT-6

coderabbitai · 2026-09-27T20:06:32Z

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: RustPython/RustPython/.coderabbit.yml

Review profile: CHILL

Plan: Advanced

Run ID: f2bbb85d-4c94-4395-9e50-5e4c91e97d75

📥 Commits

Reviewing files that changed from the base of the PR and between 456a8dc and 1587647.

📒 Files selected for processing (3)

crates/vm/src/builtins/set.rs
crates/vm/src/dict_inner.rs
extra_tests/snippets/builtin_set.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

Walkthrough

Set difference operations now clone shared storage. Symmetric difference operations reuse stored hashes for deduplicated inputs. difference_update removes items during iteration. Tests cover these behaviors, and Dict::delete_or_insert was removed.

Changes

Set operations

Layer / File(s)	Summary
Difference operations clone storage `crates/vm/src/builtins/set.rs`, `extra_tests/snippets/builtin_set.py`	`difference` and reflected subtraction clone the inner set before computing results. Tests cover multiple operands and verify that the input sets remain unchanged.
Symmetric difference reuses stored hashes `crates/vm/src/builtins/set.rs`, `crates/vm/src/dict_inner.rs`, `extra_tests/snippets/builtin_set.py`	Symmetric difference operations toggle deduplicated inputs using stored hashes. Tests cover repeated inputs, hash counts, and the in-place operation. The `Dict::delete_or_insert` wrapper was removed; `delete_or_insert_known_hash` remains.
`difference_update` removes items during iteration `crates/vm/src/builtins/set.rs`, `extra_tests/snippets/builtin_set.py`	`difference_update` removes items as the iterable yields them. Tests cover iterator errors, iteration over the set, and self-update.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Refactor

Merge Risk: ⚪ Minimal · up to 15876

The set-operation changes are mergeable after normal checks; no actionable issue remains identified.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 15876

Set operations remain within the existing runtime boundary, with no demonstrated new privilege or cross-service exposure. The meaningful risk is that callers handling an iteration error may now encounter a partially modified set.

Retained concerns

Low · architecture · observed: Generic difference_update inputs now commit removals during iteration. If iteration later raises, the receiver retains those removals rather than remaining unchanged through input acquisition; callers that rely on the former failure state must account for partial progress.

Security review details

Security Blast Radius

inferred — The demonstrated effect is confined to the receiver set and callers observing it; the mapped change does not establish a new service or authorization boundary.

Trust Boundaries and Controls

observed — Stored-hash symmetric-difference paths still use dictionary lookup with the VM and key before toggling storage; the removed convenience wrapper formerly delegated to that known-hash operation.

Resilience and Maintainability Implications

observed — There is no operation-wide rollback in the new generic-iterable removal loop: an exception exits after earlier deletions have completed.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 22.22% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ 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: reducing redundant work in set and frozenset difference operations, including hash reuse and incremental updates.
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.

Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create a new PR

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.}

youknowone

👍

Avoid redundant work in set difference operations

1587647

Assisted-by: Codex:GPT-6

youknowone approved these changes Sep 27, 2026

View reviewed changes

youknowone merged commit e2335f1 into RustPython:main Sep 27, 2026
21 checks passed

1ndahous3 mentioned this pull request Sep 28, 2026

Choose smaller difference scans and install completed intersections #8881

Merged

1 task done

1ndahous3 deleted the set_operation_paths branch September 28, 2026 16:09

Navigation Menu

Repository navigation

Avoid redundant work in set difference operations - #8869

Avoid redundant work in set difference operations#8869
youknowone merged 1 commit into
RustPython:mainfrom
1ndahous3:set_operation_paths

1ndahous3 commented Sep 27, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Sep 27, 2026 •

edited

Loading

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

youknowone left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

1ndahous3 commented Sep 27, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Performance

AI assistance

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Sep 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Security Architecture Review

❌ Failed checks (1 warning)

Uh oh!

youknowone left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

1ndahous3 commented Sep 27, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Sep 27, 2026 •

edited

Loading