Reuse stored hashes when merging dictionaries by 1ndahous3 · Pull Request #8858 · RustPython/RustPython

1ndahous3 · 2026-09-27T12:27:42Z

This PR follows our AI policy

Summary

Dictionary construction, update, union operators and unpacking reuse the source dictionary's stored key hashes. This avoids repeated __hash__ calls when copying or combining mappings, including tuple and user-defined keys.

Follow-up to #8503 and #8491, which added stored-hash reuse to related dictionary and set operations.

Performance

Release build, 512 four-item tuple keys; median of three samples.

Operation	Before	After	Speedup
Dictionary construction	38.66 µs	23.25 µs	1.66×
Copy and update overlapping keys	36.42 µs	20.45 µs	1.78×

AI assistance

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

Summary by CodeRabbit

Bug Fixes
- Dictionary merge operations now preserve an overwritten key’s identity and position, and retain values when a dictionary is merged with itself.
- Merges from custom mappings now use their keys() and __getitem__ behavior, keeping results consistent with the mapping’s exposed contents.
- Added coverage for dictionary construction, updates, merge operators, and unpacking.

Assisted-by: Codex:GPT-6

chatgpt-codex-connector · 2026-09-27T12:27:46Z

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

coderabbitai · 2026-09-27T12:28:12Z

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: da1b9eb4-eadf-495d-b68d-742af08f00ab

📥 Commits

Reviewing files that changed from the base of the PR and between f871707 and 359bb4d.

📒 Files selected for processing (3)

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

Dictionary entry traversal now exposes stored key hashes, and PyDict.merge_dict reuses them for destination membership checks and insertion. Tests cover exact-dict merge operations, self-merges, overwritten keys, and mapping hooks.

Changes

Dictionary merge hash reuse

Layer / File(s)	Summary
Expose stored hashes during entry traversal `crates/vm/src/dict_inner.rs`	`next_entry_with_hash` returns each live entry’s stored hash with its key and value. The `insert_known_hash` documentation identifies valid hash sources and explains that an incorrect hash can make a key unreachable.
Reuse stored hashes in dictionary merges `crates/vm/src/builtins/dict.rs`, `extra_tests/snippets/builtin_dict.py`	`merge_dict` uses stored hashes for destination membership checks and insertion. Tests cover exact-dict merge operations, self-merges, overwritten-key identity and position, and mapping hooks.

Priority: ⬇️ Low

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

Change: Refactor

Merge Risk: ⚪ Minimal · up to 359bb

No merge-blocking issue was established in the dictionary merge changes. Merge after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 359bb

Dictionary merges now reuse stored key hashes instead of calling the key’s hash method again. This changes observable behavior and warrants review of dictionary correctness, but the reviewed paths show no new privilege or trust boundary.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

inferred — The changed behavior is reachable through VM dictionary operations on exact dictionaries. The reviewed call boundary does not show a new credential, tenant, or service authority transition.

Trust Boundaries and Controls

observed — The exact-dictionary check limits stored-hash reuse to entries owned by internal dictionary storage. Equality callbacks execute after the lookup read guard is released.

Resilience and Maintainability Implications

observed — The merge retains its final source-size mutation check and error; that check does not by itself detect a callback that restores the recorded size.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 15.38% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 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: reusing stored hashes during dictionary merges.
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.}

codspeed · 2026-09-27T14:01:43Z

Merging this PR will not alter performance

✅ 62 untouched benchmarks
⏩ 4 skipped benchmarks¹

_{Comparing 1ndahous3:dict_merge_hashes (359bb4d) with main (f871707)}

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

Reuse stored hashes when merging dictionaries

359bb4d

Assisted-by: Codex:GPT-6

youknowone approved these changes Sep 27, 2026

View reviewed changes

youknowone enabled auto-merge (squash) September 27, 2026 15:06

youknowone merged commit ad8bfdb into RustPython:main Sep 27, 2026
22 checks passed

1ndahous3 deleted the dict_merge_hashes branch September 27, 2026 17:40

youknowdot mentioned this pull request Oct 10, 2026

Reuse stored dict hashes when merging call keywords #9053

Merged

Navigation Menu

Repository navigation

Reuse stored hashes when merging dictionaries - #8858

Reuse stored hashes when merging dictionaries#8858
youknowone merged 1 commit into
RustPython:mainfrom
1ndahous3:dict_merge_hashes

1ndahous3 commented Sep 27, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

chatgpt-codex-connector Bot commented Sep 27, 2026

Uh oh!

coderabbitai Bot commented Sep 27, 2026 •

edited

Loading

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

codspeed Bot commented Sep 27, 2026

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!

chatgpt-codex-connector Bot commented Sep 27, 2026

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!

codspeed Bot commented Sep 27, 2026

Merging this PR will not alter performance

Footnotes

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