Add sort.py to microbenchmarks #6086
Conversation
WalkthroughA new Python benchmark script for sorting random floats was added, and the benchmarking README was updated to include instructions for running specific benchmarks by name using Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
benches/README.md (1)
10-16: Extend example to include list.sort() microbench and show exact-match usageOnce you add the in-place list.sort() microbenchmark, surface it here. Also consider an exact-match example to avoid unintentionally matching other benchmarks.
Apply:
```shell -cargo bench sort +cargo bench sort +# Or, for the in-place list.sort() microbenchmark: +cargo bench list_sort +# For an exact match on just "sort": +cargo bench '^sort$'</blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: .coderabbit.yml** **Review profile: CHILL** **Plan: Pro** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between a9a9e3bf118f6c631ef99fe7174215254f846097 and d96712bd950005d8d813bd3a4d8f44fb0426ec58. </details> <details> <summary>📒 Files selected for processing (2)</summary> * `benches/README.md` (1 hunks) * `benches/microbenchmarks/sort.py` (1 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>📓 Path-based instructions (1)</summary> <details> <summary>**/*.py</summary> **📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)** > `**/*.py`: Follow PEP 8 style for custom Python code > Use ruff for linting Python code Files: - `benches/microbenchmarks/sort.py` </details> </details><details> <summary>🧠 Learnings (1)</summary> <details> <summary>📚 Learning: 2025-07-19T03:16:56.511Z</summary>Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-19T03:16:56.511Z
Learning: Generate documentation withcargo doc --no-deps --all**Applied to files:** - `benches/README.md` </details> </details><details> <summary>🧬 Code Graph Analysis (1)</summary> <details> <summary>benches/microbenchmarks/sort.py (2)</summary><blockquote> <details> <summary>stdlib/src/random.rs (1)</summary> * `random` (42-45) </details> <details> <summary>vm/src/stdlib/builtins.rs (1)</summary> * `sorted` (815-820) </details> </blockquote></details> </details><details> <summary>🪛 Flake8 (7.2.0)</summary> <details> <summary>benches/microbenchmarks/sort.py</summary> [error] 3-3: undefined name 'ITERATIONS' (F821) </details> </details> </details> <details> <summary>⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)</summary> * GitHub Check: Run snippets and cpython tests (macos-latest) * GitHub Check: Run snippets and cpython tests on wasm-wasi * GitHub Check: Run snippets and cpython tests (windows-latest) * GitHub Check: Run snippets and cpython tests (ubuntu-latest) * GitHub Check: Run tests under miri * GitHub Check: Check the WASM package and demo * GitHub Check: Check Rust code with rustfmt and clippy * GitHub Check: Run rust tests (ubuntu-latest) * GitHub Check: Run rust tests (windows-latest) * GitHub Check: Run rust tests (macos-latest) * GitHub Check: Ensure compilation on various targets </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
I chose 5 * Iterations to try better show that RustPython sort implementation scales noticeably worse CPython's with respect to the number of elements.
d96712b to
5531ae2
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
benches/microbenchmarks/sort.py (1)
1-1: Ensure Consistent RNG Seeding Across MicrobenchmarksNo other benchmarks in
benches/microbenchmarks/currently seed the RNG. If you’d like deterministic inputs for easier cross-run comparisons, consider adding a fixed seed at the top ofbenches/microbenchmarks/sort.py:-from random import random +from random import random, seed +seed(0)This change will align
sort.pywith a reproducible input setup—apply only if it matches the rest of your benchmarking strategy.
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
benches/README.md(1 hunks)benches/microbenchmarks/sort.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- benches/README.md
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.py: Follow PEP 8 style for custom Python code
Use ruff for linting Python code
Files:
benches/microbenchmarks/sort.py
🧠 Learnings (3)
📓 Common learnings
Learnt from: jackoconnordev
PR: RustPython/RustPython#6086
File: benches/microbenchmarks/sort.py:7-7
Timestamp: 2025-08-09T22:55:10.479Z
Learning: In RustPython benchmarks, when testing sorting performance, benchmarking only `sorted()` is sufficient since it internally uses the same sorting logic as `list.sort()`. The only difference is that `sorted()` creates a copy first, so testing both would be redundant for evaluating the sorting algorithm's performance.
📚 Learning: 2025-08-09T22:55:10.479Z
Learnt from: jackoconnordev
PR: RustPython/RustPython#6086
File: benches/microbenchmarks/sort.py:7-7
Timestamp: 2025-08-09T22:55:10.479Z
Learning: In RustPython benchmarks, when testing sorting performance, benchmarking only `sorted()` is sufficient since it internally uses the same sorting logic as `list.sort()`. The only difference is that `sorted()` creates a copy first, so testing both would be redundant for evaluating the sorting algorithm's performance.
Applied to files:
benches/microbenchmarks/sort.py
📚 Learning: 2025-08-09T22:56:24.504Z
Learnt from: jackoconnordev
PR: RustPython/RustPython#6086
File: benches/microbenchmarks/sort.py:1-3
Timestamp: 2025-08-09T22:56:24.504Z
Learning: In RustPython's microbenchmarks (benches/microbenchmarks/*.py), the variable `ITERATIONS` is intentionally used without being defined in the Python files. It is injected by the cargo bench harness at runtime. This pattern should be maintained for consistency across all microbenchmarks, and F821 lint warnings for undefined `ITERATIONS` are expected and acceptable in this context.
Applied to files:
benches/microbenchmarks/sort.py
🧬 Code Graph Analysis (1)
benches/microbenchmarks/sort.py (2)
stdlib/src/random.rs (1)
random(42-45)vm/src/builtins/list.rs (1)
sort(323-337)
🪛 Flake8 (7.2.0)
benches/microbenchmarks/sort.py
[error] 3-3: undefined name 'ITERATIONS'
(F821)
🔇 Additional comments (2)
benches/microbenchmarks/sort.py (2)
3-3: ITERATIONS is intentionally harness-injected; keep as-isAcknowledging the established pattern in RustPython microbenchmarks: ITERATIONS is provided by the bench harness. F821 from local linters is expected here and acceptable. No change requested.
7-7: No change required: setup is re-executed per iteration
The harness uses Criterion’siter_batched_ref/iter_batchedAPIs with thebench_setupclosure, which runs the setup block (includingunsorted_list = […]) before each sample. That guarantees each sort operates on a fresh, random list.• In
benches/microbenchmarks.rs, see lines 78–86 (b.iter_batched_ref(|| bench_setup(Some(*idx)), …)) and lines 147–155 (b.iter_batched(|| bench_setup(Some(*idx)), …))
• Each invocation of the setup closure re-executes the code before timing, so no iteration ever sorts an already-sorted list.
Description
sorted(and by extensionsort) builtin functionsI chose
5 * ITERATIONSto better show the divergence between the sort implementations, while trying to avoid individual microbenchmark runs timing out.Sample benchmark results
Violin plot
Line Chart
Manually benchmark
Using larger list sizes really shows the difference. Sorting 1_000_000 random numbers:
Summary by CodeRabbit