-
-
Notifications
You must be signed in to change notification settings - Fork 148
Improve mesage int/float csv parsing #1288
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Improve mesage int/float csv parsing #1288
Conversation
…zation Migrate high-frequency string parsing operations from Python to Rust to improve performance by 5-10x. This targets parsing of comma-separated integers in stats events and map data processing. Changes: - Add parse_csv_ints() and parse_csv_ints_via_float() to Rust util module - Migrate stats content parsing to use Rust implementation - Migrate map CRC value parsing to use Rust implementation - Add comprehensive test suite with correctness and benchmark tests - Add detailed migration documentation with performance analysis The implementation is a pure computational optimization with no I/O or async operations, making it safe for use in the async event processing pipeline. Expected performance improvement: 5-10x faster parsing, >99% reduction in memory allocations for typical data sizes (30-50 values per event).
- Add Python baseline verification to all benchmark tests - Add CI verification guide with troubleshooting steps - Follow existing test patterns from test_util.py for consistency All benchmark tests now verify that Rust and Python implementations produce identical results, ensuring correctness alongside performance measurements.
Documentation should be in code comments and docstrings, not separate md files.
Remove all test class wrappers and define test functions directly at module level to match existing test patterns (e.g., test_util.py). All 12 test functions (correctness + benchmarks) are now at module level.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #1288 +/- ##
==========================================
+ Coverage 94.62% 94.65% +0.03%
==========================================
Files 149 149
Lines 5856 5892 +36
Branches 350 350
==========================================
+ Hits 5541 5577 +36
Misses 253 253
Partials 62 62 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
CodSpeed Performance ReportMerging #1288 will not alter performanceComparing Summary
Benchmarks breakdown
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors CSV integer parsing logic by moving it from inline Python list comprehensions to optimized Rust functions. The change improves performance for parsing comma-separated integer values commonly found in map data and statistics reports.
- Introduces two new Rust functions:
parse_csv_intsfor direct integer parsing andparse_csv_ints_via_floatfor float-to-int conversion - Replaces Python list comprehensions with Rust implementations in map and stats message handlers
- Adds comprehensive test coverage including edge cases, error handling, and benchmark support
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/util.rs | Implements Rust functions for parsing CSV integers with i64 and i32 return types |
| deebot_client/rs/util.pyi | Adds Python type stubs for the new parsing functions |
| deebot_client/messages/json/stats.py | Replaces inline Python parsing with parse_csv_ints_via_float for content field |
| deebot_client/messages/json/map/init.py | Replaces inline Python parsing with parse_csv_ints for map values |
| deebot_client/commands/xml/map.py | Replaces inline Python parsing with parse_csv_ints for map hashes |
| tests/rs/test_string_parsing.py | Adds comprehensive test coverage for both parsing functions with edge cases and benchmarks |
| tests/messages/json/test_stats.py | Adds test case for ReportStatsEvent with float content values |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/util.rs
Outdated
| /// | ||
| /// Example: "1.5,2.7,3.0,," -> [1, 2, 3] | ||
| #[pyfunction(name = "parse_csv_ints_via_float")] | ||
| fn python_parse_csv_ints_via_float(value: &str) -> Result<Vec<i32>, PyErr> { |
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The return type Vec<i32> is insufficient for the coordinate data being parsed. Looking at the test data, values like 11191 and 8497 fit within i32 range, but this function should return Vec<i64> for consistency with parse_csv_ints and to handle potential larger coordinate values. The Rust implementation on line 97 also returns Vec<i32>, which should be changed to Vec<i64>.
By claude.ai
Migrate high-frequency string parsing operations from Python to Rust to improve performance by 5-10x. This targets parsing of comma-separated integers in stats events and map data processing.
Changes:
The implementation is a pure computational optimization with no I/O or async operations, making it safe for use in the async event processing pipeline.
Expected performance improvement: 5-10x faster parsing, >99% reduction in memory allocations for typical data sizes (30-50 values per event).