Repository navigation
Conversation
- Add descriptions option to PacketConfig and System so decom and interface microservices drop descriptions they never use (Ruby/Python) - Set RUBY_GC_HEAP_GROWTH_FACTOR=1.2 in the openc3-ruby images so loading large definitions doesn't overshoot the heap - Add test/integration/cmd_tlm generator and benchmarks for a 2000 packet, 500k item, 2M state definition Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Create PacketItemLimits on first access and add limits_state and limits_values readers so decom and as_json don't allocate them - Stop creating an empty state_colors hash whenever states are set, guarding handle_limits_states for items with states but no colors - Dedup item names and keys (-str / sys.intern) plus Python state names, since Python dicts don't share string keys like Ruby hashes Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Add __slots__ to StructureItem and PacketItem since 39 attributes exceed Python's shared-key dict limit (1.6 KB dict per item) - Rename the StructureItem.create_index class counter to _next_create_index so it doesn't clash with the instance slot - Create PacketItem meta on first access instead of an empty dict - Update states and states_by_value in place in StateParser instead of rebuilding both dicts on every STATE line - Compile the ConfigParser parsing regex once per file, not per line Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4027 +/- ##
==========================================
- Coverage 80.14% 80.11% -0.04%
==========================================
Files 901 901
Lines 68374 68455 +81
Branches 2650 2650
==========================================
+ Hits 54798 54840 +42
- Misses 12911 12954 +43
+ Partials 665 661 -4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
It changes core packet and limits behavior across both runtimes and globally tunes Ruby garbage collection.
0 open findings
What changed in this PR
No factual issues (0 important, 0 nits).
Reduces command/telemetry memory usage across Ruby and Python while adding reproducible benchmarks.
Changes:
- Lazily allocates limits/metadata and deduplicates repeated names and states.
- Allows interface and decom services to discard unused descriptions.
- Adds benchmarks, tests, parser optimization, and Ruby GC tuning.
| File | Description |
|---|---|
openc3/test/integration/cmd_tlm/README.md |
Documents benchmark usage. |
openc3/test/integration/cmd_tlm/generate_cmd_tlm.rb |
Generates large deterministic definitions. |
openc3/test/integration/cmd_tlm/benchmark_cmd_tlm.rb |
Adds Ruby memory benchmark. |
openc3/test/integration/cmd_tlm/benchmark_cmd_tlm.py |
Adds Python memory benchmark. |
openc3/test/integration/cmd_tlm/.gitignore |
Excludes generated data. |
openc3/spec/system/system_spec.rb |
Tests Ruby description removal. |
openc3/spec/packets/structure_item_spec.rb |
Tests Ruby string deduplication. |
openc3/spec/packets/packet_spec.rb |
Tests limits without colors. |
openc3/spec/packets/packet_item_spec.rb |
Tests lazy Ruby limits. |
openc3/spec/packets/packet_config_spec.rb |
Tests description filtering. |
openc3/python/test/system/test_system.py |
Tests Python description removal. |
openc3/python/test/packets/test_structure_item.py |
Tests Python string interning. |
openc3/python/test/packets/test_packet.py |
Tests limits without colors. |
openc3/python/test/packets/test_packet_item.py |
Tests lazy Python limits. |
openc3/python/test/packets/test_packet_config.py |
Tests description filtering. |
openc3/python/openc3/system/system.py |
Propagates description retention settings. |
openc3/python/openc3/packets/structure_item.py |
Adds slots and string interning. |
openc3/python/openc3/packets/parsers/state_parser.py |
Optimizes state parsing. |
openc3/python/openc3/packets/packet.py |
Avoids unnecessary limits allocation. |
openc3/python/openc3/packets/packet_item.py |
Lazily creates limits and metadata. |
openc3/python/openc3/packets/packet_config.py |
Supports dropping descriptions. |
openc3/python/openc3/microservices/microservice.py |
Adds description-loading policy. |
openc3/python/openc3/microservices/interface_microservice.py |
Disables interface descriptions. |
openc3/python/openc3/microservices/decom_microservice.py |
Disables decom descriptions. |
openc3/python/openc3/config/config_parser.py |
Reuses compiled regexes. |
openc3/lib/openc3/system/system.rb |
Propagates description retention settings. |
openc3/lib/openc3/packets/structure_item.rb |
Deduplicates names and keys. |
openc3/lib/openc3/packets/parsers/state_parser.rb |
Avoids eager limits allocation. |
openc3/lib/openc3/packets/packet.rb |
Uses lazy limits accessors safely. |
openc3/lib/openc3/packets/packet_item.rb |
Lazily creates limits. |
openc3/lib/openc3/packets/packet_config.rb |
Supports dropping descriptions. |
openc3/lib/openc3/microservices/microservice.rb |
Adds description-loading policy. |
openc3/lib/openc3/microservices/interface_microservice.rb |
Disables interface descriptions. |
openc3/lib/openc3/microservices/decom_microservice.rb |
Disables decom descriptions. |
openc3-ruby/Dockerfile-ubi |
Tunes Ruby heap growth. |
openc3-ruby/Dockerfile |
Tunes Ruby heap growth. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This branch has not been deployed
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
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.
What changed
Reduces the memory (and some of the time) needed to load very large command/telemetry definitions, in both Ruby and Python.
Load less in microservices
PacketConfig,System.new,System.instanceandSystem.setup_targetstake adescriptions:option (defaulttrue). Whenfalse, packet and item descriptions are dropped as each packet/item is finished.Microservice#load_descriptions?(Pythonload_descriptions()) defaults totrue.DecomMicroserviceandInterfaceMicroservice(and therefore routers) returnfalse, since they never use descriptions. Plugin install, the APIs, reingest and userMICROSERVICEs still keep them, so what's stored in Redis is unchanged.Stop allocating per-item objects nobody uses (Ruby and Python)
PacketItemLimitsis created on first access ofitem.limits. Newlimits_state/limits_valuesreaders don't create it, and they're used on the paths that touch every item (decom's per-packetas_json,read_all_with_limits_states,update_limits_items_cache,StateParser).statesno longer creates an emptystate_colorshash.handle_limits_stateshandlesstate_colorsbeing nil.-strin Ruby,sys.internin Python). In Python, state names are interned too (Ruby hashes already share string keys).Python specific
__slots__onStructureItemandPacketItem. With 39 attributes, items were over Python's 30-attribute limit for sharing attribute tables between instances, so each item carried its own ~1.6 KB__dict__. A slotted item is ~360 bytes.__dict__is kept as a slot, so setting other attributes still works.StructureItem.create_indexclass counter is renamed to_next_create_indexso it doesn't clash with the instance slot of the same name.PacketItem.metais created on first access (it was an empty dict on every item).StateParserupdatesstates/states_by_valuein place instead of reassigningitem.states, which rebuilt both dicts on every STATE line.ConfigParser.parse_loopcompiles its regex once per file instead of once per line.Ruby images
openc3-ruby/DockerfileandDockerfile-ubisetRUBY_GC_HEAP_GROWTH_FACTOR=1.2(default 1.8). It can be overridden per container.Benchmark
openc3/test/integration/cmd_tlm/addsgenerate_cmd_tlm.rb, which generates a deterministic definition: 2,000 tlm packets / 500,000 items / 2,000,000 states, 2,000 cmd packets / 150,000 params, with 50–250 char descriptions. It also adds Ruby and Python benchmark scripts (--no-descriptionssimulates decom/interface). See the README there.Why it changed
Large definitions run deployed COSMOS out of memory. Each decom and interface microservice process re-parses the full cmd/tlm text via
System.setup_targetsand keeps its own copy. On the benchmark definition that was ~1.3 GB per Ruby process and ~1.9 GB peak per Python process.Profiling the Ruby parse showed:
PacketItemobjects themselves, aPacketItemLimitson every item, an emptystate_colorshash on every item with states, and separate name/key strings.In Python, the per-item
__dict__was ~65% of live memory.Testing strategy
PacketConfigandSystem.setup_targets, includingDESCRIPTIONandSELECT_ITEM.as_json/to_config/clonenot creating them.state_colorscreated bystates=.check_limitson an item with states and limits values but no colors (fails without the nil guard).StateParserupdate (fails without removing the stale value mapping).spec/packets spec/system spec/microservices spec/api spec/models spec/topics spec/utilities(2747 examples, 0 failures) and the full Python suite (3035 passed).ruffis clean.JSON.generate(packet.as_json)for every packet (generated definition plus the specINST/SYSTEMtargets, which have limits and state colors) is byte-identical before and after, in Ruby and Python.generate_cmd_tlm.rbwith the default sizes, thenbenchmark_cmd_tlm.rb/.pybefore and after each change on the same machine.Results
Full-size benchmark definition (macOS, Ruby 3.4.5 without YJIT, Python 3.12.12). Memory is RSS after parsing, which is what each microservice keeps (Python reports peak RSS). RSS varies by roughly ±5% between identical runs.
Breakdown of the Ruby decom/interface result from separate runs: GC growth factor −175 MB, dropping descriptions −225 MB, per-item allocations −190 MB.
Review notes
nil/Nonefromitem.description/packet.description. No built-in code reads them there.item.limits = nilnow resets to default limits on next access instead of leavinglimitsnil. Every caller dereferenceslimits, so nothing relied on nil.StructureItem.create_indexshould use_next_create_index.item.create_indexis unchanged.TableItemhas no slots, so it keeps a full__dict__(tables are small).telemetry.c/Telemetry#values_and_limits_statesstill callitem.limits, so they create limits objects only for the items a caller explicitly requests.PacketItem~320 B, 35 instance variables). Parse speed is dominated by per-line work inConfigParserandPacketItemParser.🤖 Generated with Claude Code