Optimize command and telemetry decom performance in Ruby and Python#2726
Merged
Optimize command and telemetry decom performance in Ruby and Python#2726
Conversation
- Remove clone.freeze from received_time= in C extension and Ruby - Replace JSON deep copy with deep_dup for packet.extra in clone - Skip buffer.dup in internal_buffer_equals when buffer is frozen - Add performance benchmarking spec for InterfaceMicroservice Performance improvements: - identify!: +16% faster - update!: +15% faster - Memory allocation: -70% reduction Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Inline packet lookup in build_cmd to avoid redundant upcase calls - Use write_item directly in set_parameters to skip redundant get_item - Change RESERVED_ITEM_NAMES from Array to Set for O(1) lookup - Use batch read_items and given_raw in CommandDecomTopic to avoid re-reading values from buffer Benchmarks show ~7.3% improvement in full command path (134.57 -> 124.79 μs) Co-Authored-By: Claude Opus 4.5 <[email protected]>
…tion - Add CvtModel.set_json method that accepts pre-serialized JSON - Call as_json once in TelemetryDecomTopic.write_packet and reuse result - Eliminates double as_json calls that were serializing same data twice Benchmarks show ~24% improvement in TelemetryDecomTopic.write_packet (228 -> 173 μs) and ~26% improvement in full decom path (242 -> 179 μs) Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Eliminate redundant JSON serialization in TelemetryDecomTopic by reusing pre-serialized JSON for both topic write and CVT set - Add CvtModel.set_json() method to accept pre-serialized JSON - Convert RESERVED_ITEM_NAMES and VALUE_TYPES from list to set for O(1) lookup - Inline packet lookup in Commands.build_cmd to match Ruby implementation - Use given_raw parameter in CommandDecomTopic to match Ruby implementation Add Python performance benchmark tests for command and telemetry paths. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2726 +/- ##
==========================================
+ Coverage 79.22% 79.23% +0.01%
==========================================
Files 670 670
Lines 54178 54206 +28
Branches 734 734
==========================================
+ Hits 42923 42952 +29
+ Misses 11175 11174 -1
Partials 80 80
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
jmthomas
commented
Jan 10, 2026
| json_hash[item.name + "__C"] = packet.read_item(item, :CONVERTED, packet.buffer, given_raw) if item.write_conversion or item.states | ||
| json_hash[item.name + "__F"] = packet.read_item(item, :FORMATTED, packet.buffer, given_raw) if item.format_string | ||
| json_hash[item.name + "__U"] = packet.read_item(item, :WITH_UNITS, packet.buffer, given_raw) if item.units | ||
| end |
Member
Author
There was a problem hiding this comment.
This same logic is present in the packet decom method so I think it makes sense here
| raise(ArgumentError, "received_time must be a Time but is a #{received_time.class}") | ||
| end | ||
| @received_time = received_time.clone.freeze | ||
| @received_time = received_time |
Member
Author
There was a problem hiding this comment.
Not sure why we were cloning and freezing here. Claude's analysis of received_time indicate there are no mutations found after assignment.
| # Also update the current value table with the latest decommutated data | ||
| CvtModel.set(json_hash, target_name: packet.target_name, packet_name: packet.packet_name, scope: scope) | ||
| # Pass pre-serialized JSON to avoid calling as_json twice | ||
| CvtModel.set_json(json_data, json_safe_hash, target_name: packet.target_name, packet_name: packet.packet_name, scope: scope) |
Member
Author
There was a problem hiding this comment.
This was a big optimization since we're not calling as_json twice
- Make performance tests opt-in via PERFORMANCE=1 env var instead of excluding them via CI env var - Reduce excessive sleep times in protocol and microservice specs - Optimize limits_api_spec.rb: wrap DecomMicroservice setup in helper, only 3 of 36 tests actually need it (~33s saved) - Optimize commands_spec.rb: setup InterfaceCmdHandlerThread only for connected tests that need it (~46s saved) - Reduce sleep times in packet_log_writer, stream_log, template_protocol, cmd_response_protocol, and router_microservice specs Total improvement: ~98 seconds saved (4:38 -> 3:00) Co-Authored-By: Claude Opus 4.5 <[email protected]>
- test_handles_exceptions_in_user_processors: Add retry loop to wait for async decom processing instead of fixed 10ms sleep - test_is_single_threaded: Fix variable shadowing bug where 'for threads in threads' caused incorrect thread joining, and relax timing threshold from 1.0s to 0.9s for CI variability Co-Authored-By: Claude Opus 4.5 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Ruby Optimizations
build_cmdto avoid repeated method callsgiven_rawparameter inCommandDecomTopic.write_packetto avoid re-reading from bufferTelemetryDecomTopicby reusing pre-serialized JSONCvtModel.set_jsonmethod to accept pre-serialized JSONclone.freezeonreceived_timePython Optimizations
TelemetryDecomTopic(main improvement)CvtModel.set_json()method to accept pre-serialized JSONRESERVED_ITEM_NAMESandVALUE_TYPESfrom list to set for O(1) lookupCommands.build_cmdto match Ruby implementationgiven_rawparameter inCommandDecomTopicto match Ruby implementationThroughput Summary (Ruby vs Python)
Test plan
🤖 Generated with Claude Code