Fix loop time spikes from GC pressure and NT/CAN overhead - #6
Open
ronald-mercer-island wants to merge 1 commit into
Open
Fix loop time spikes from GC pressure and NT/CAN overhead#6ronald-mercer-island wants to merge 1 commit into
ronald-mercer-island wants to merge 1 commit into
Conversation
Root causes and fixes: 1. LoggedInterpolatingTable: getTableFromDashboard() allocated a new TreeMap on every get()/getMinKey()/getMaxKey() call independently. In Shooter.periodic() with a 5-iteration aiming loop querying 6 tables, this caused ~30+ TreeMap allocations per 20ms loop — triggering periodic GC spikes. Fix: rebuild cache once per loop in periodic() via a new activeTable() helper; all read methods share the single cached instance. 2. Constants: kTuningMode=true and kEnableLoopTimingLogs=true were left on in production. kTuningMode drives all LoggedTunableNumber NT reads (~100+/loop) and was the prerequisite for the TreeMap allocation bug. kEnableLoopTimingLogs added ~20 Timer.getFPGATimestamp() calls per loop and was declared final preventing runtime override. Both default to false with explanatory comments; set to true only during tuning sessions. 3. AngularIOTalonFX: acceleration, referencePosition, and referenceVelocity signals were registered at 50 Hz but are logging-only (not used in any control loop). With 7 non-drive subsystems, that was 1050 unnecessary CAN signal updates/second on the RIO bus. Reduced to 20 Hz. 4. AngularSubsystem: disconnect check created Stream objects every loop via Arrays.stream().allMatch() and stream-based string building, for all subsystems. Replaced with plain for-each loops. Same fix applied to areAllDevicesConnected(). 5. VisionIOLimelight: updateInputs() allocated new HashSet and ArrayList on every call across 5 cameras (10+ allocations/loop). Pre-allocated as fields; cleared and reused each call. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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
2 participants
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.
Root causes and fixes:
LoggedInterpolatingTable: getTableFromDashboard() allocated a new TreeMap on every get()/getMinKey()/getMaxKey() call independently. In Shooter.periodic() with a 5-iteration aiming loop querying 6 tables, this caused ~30+ TreeMap allocations per 20ms loop — triggering periodic GC spikes. Fix: rebuild cache once per loop in periodic() via a new activeTable() helper; all read methods share the single cached instance.
Constants: kTuningMode=true and kEnableLoopTimingLogs=true were left on in production. kTuningMode drives all LoggedTunableNumber NT reads (~100+/loop) and was the prerequisite for the TreeMap allocation bug. kEnableLoopTimingLogs added ~20 Timer.getFPGATimestamp() calls per loop and was declared final preventing runtime override. Both default to false with explanatory comments; set to true only during tuning sessions.
AngularIOTalonFX: acceleration, referencePosition, and referenceVelocity signals were registered at 50 Hz but are logging-only (not used in any control loop). With 7 non-drive subsystems, that was 1050 unnecessary CAN signal updates/second on the RIO bus. Reduced to 20 Hz.
AngularSubsystem: disconnect check created Stream objects every loop via Arrays.stream().allMatch() and stream-based string building, for all subsystems. Replaced with plain for-each loops. Same fix applied to areAllDevicesConnected().
VisionIOLimelight: updateInputs() allocated new HashSet and ArrayList on every call across 5 cameras (10+ allocations/loop). Pre-allocated as fields; cleared and reused each call.