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.

• Autopilot · 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.

• X
• Mastodon
• Reddit
• LinkedIn

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

Merging this PR will improve performance by 11.1%

⚡ 1 improved benchmark
✅ 61 untouched benchmarks
⏩ 4 skipped benchmarks1

Performance Changes

Comparing dependabot/cargo/which-8.0.6 (da49a23) with main (3e0e401)

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

@codspeedbot explain why performance improved

@codspeedbot explain why performance improved

Why rustpython[loop_string.py] improved by 11.1%

Short answer: it's measurement noise, not a real speedup. The which bump didn't cause it. The PR head is at the usual level for this benchmark, and the main baseline it was compared against (3e0e401) happened to be a slow run.

This PR doesn't touch the benchmarked code

• The only change is Cargo.lock (which 8.0.5 → 8.0.6).
• which has one caller: host_env::fs::which, used by getpath.rs to find the executable when the interpreter starts. That runs outside the measured routine. loop_string.py only iterates over a string (for char in string: pass).

This benchmark moves around on main too

Recent main commits, measured in Simulation mode:

The PR head matches the usual ~845 µs. The base is the highest value in the window.

Where the difference shows up in the profile

Comparing the two flamegraphs, almost all of the 93.6 µs difference is in one function:

Everything else is identical to within a few hundred ns, including ExecutingFrame::run self time, execute_for_iter, PyStrIterator and hash_or_unhashable. Dict::lookup here runs inside STORE_NAME for the loop variable char, which is written into the module globals dict on every iteration.

Root cause: the string hash seed is random in each run

benches/microbenchmarks.rs builds the interpreter from Settings::default(), which leaves hash_seed: None. init_hash_secret then picks a random hash secret for each process, so the hash of "char" changes from run to run. That changes how far the lookup has to probe in the globals dict before it finds the key. Some runs land on a longer chain, and since that lookup happens on every loop iteration, a few extra probes show up as about 10% on this benchmark. The base and head differ in cache cost as well as instruction count, which also points to the dict layout rather than any code change.

Suggested fix (not applied here)

To make dict-heavy microbenchmarks repeatable, pin the seed in the bench harness:

// benches/microbenchmarks.rs (and benches/execution.rs)
let mut settings = Settings::default();
settings.hash_seed = Some(0);

I haven't pushed this, because it doesn't belong in a Dependabot PR. I can open a separate PR for it if you'd like.

Verdict: this PR has no performance impact either way and is safe to merge.

Comparison on CodSpeed

• Warming up the performance wizard...
• Handling comment — Comment handling finished successfully

Learnings updated. See what CodSpeed remembers about this repository.

Was this response helpful? React with 👍 or 👎 to help us improve CodSpeedBot.

@youknowone see above