Perf investigation: GTFS-RT matching CPU harness + findings#472
Merged
Conversation
Also wires the two GtfsRealtimeSource dependencies PerfBundleHarness was missing (VehicleOccupancyListener, ServiceAlertsService). Without them refresh() NPEs on the first vehicle occupancy record and again on the first alert, since those setters are normally @Autowired by Spring but this harness builds GtfsRealtimeSource by hand. VehicleOccupancyListener reuses the existing VehicleStatusServiceImpl bean (it implements both listener interfaces); ServiceAlertsService gets a minimal in-memory implementation since the DB-backed one isn't wired into this harness's Spring context.
…date Locks the observable output of GtfsRealtimeTripLibrary's matching hot path (blockId, tripId, serviceDate, scheduleDeviation, timepoint predictions) against a committed golden fixture, so upcoming allocation-reduction changes to BlockStopTimeList.get() and friends can be verified as behavior-preserving. Fixture construction is ported from GtfsRealtimeTripLibraryTest#testTprInterpolation_0(), the existing case that produces multiple timepoint predictions (one interpolated, one from the feed). Regenerate with -Dupdate.golden=true.
📝 WalkthroughWalkthroughAdds a standalone GTFS-RT performance module with bundle setup, refresh benchmarking, CPU/allocation profiling, stop-count analysis, a golden characterization test, and investigation documentation. Matching engine implementation is unchanged. ChangesGTFS-RT performance investigation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Benchmark
participant Harness
participant GtfsRealtimeSource
participant Listener
Benchmark->>Harness: refreshOnce()
Harness->>GtfsRealtimeSource: reset()
Harness->>GtfsRealtimeSource: refresh()
GtfsRealtimeSource->>Listener: vehicle-location callbacks
Listener-->>Harness: record count
Benchmark-->>Benchmark: aggregate timed samples
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
onebusaway-transit-data-federation/src/test/java/org/onebusaway/transit_data_federation/impl/realtime/gtfs_realtime/GtfsRealtimeTripLibraryCharacterizationTest.java (1)
91-106: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove duplicate
setBlockCalendarServicecall on mock.Lines 101 and 105 are identical no-op calls to
setBlockCalendarServiceon a Mockito mock. The mock is already properly configured viawhen().thenReturn()on line 99, so both setter calls are unnecessary — and line 105 is a straight duplicate of line 101.♻️ Proposed cleanup
Mockito.when(_serviceSource.getBlockCalendarService()).thenReturn(_blockCalendarService); Mockito.when(_serviceSource.getBlockFinder()).thenReturn(new BlockFinder(_serviceSource)); - _serviceSource.setBlockCalendarService(_blockCalendarService); _library.setEntitySource(_entitySource); _library.setServiceSource(_serviceSource); - - _serviceSource.setBlockCalendarService(_blockCalendarService); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@onebusaway-transit-data-federation/src/test/java/org/onebusaway/transit_data_federation/impl/realtime/gtfs_realtime/GtfsRealtimeTripLibraryCharacterizationTest.java` around lines 91 - 106, Remove the redundant duplicate invocation of setBlockCalendarService in the before() setup method; retain only the necessary mock configuration and single setter call, or remove both if the setter is not required for test behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@onebusaway-gtfsrt-perf/src/main/java/org/onebusaway/perf/gtfsrt/PerfBundleHarness.java`:
- Around line 159-160: Delegate handleRawPosition in the listener adapter to the
wrapped real listener, passing through vehicleId, lat, lon, and timestamp just
like resetVehicleLocation delegates to _delegate, so VehicleStatusServiceImpl
processes every raw position update during performance runs.
---
Nitpick comments:
In
`@onebusaway-transit-data-federation/src/test/java/org/onebusaway/transit_data_federation/impl/realtime/gtfs_realtime/GtfsRealtimeTripLibraryCharacterizationTest.java`:
- Around line 91-106: Remove the redundant duplicate invocation of
setBlockCalendarService in the before() setup method; retain only the necessary
mock configuration and single setter call, or remove both if the setter is not
required for test behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e6a5857c-de7e-40c3-9c5d-8567a922c22c
📒 Files selected for processing (13)
docs/perf/2026-07-10-gtfsrt-matching-findings.mddocs/superpowers/plans/2026-07-10-gtfsrt-matching-perf-harness.mddocs/superpowers/specs/2026-07-10-gtfsrt-matching-perf-investigation-design.mdonebusaway-gtfsrt-perf/pom.xmlonebusaway-gtfsrt-perf/src/main/java/org/onebusaway/perf/gtfsrt/BuildKcmBundle.javaonebusaway-gtfsrt-perf/src/main/java/org/onebusaway/perf/gtfsrt/GtfsRtRefreshBenchmark.javaonebusaway-gtfsrt-perf/src/main/java/org/onebusaway/perf/gtfsrt/PerfBundleHarness.javaonebusaway-gtfsrt-perf/src/main/java/org/onebusaway/perf/gtfsrt/Profiling.javaonebusaway-gtfsrt-perf/src/main/java/org/onebusaway/perf/gtfsrt/StopCountHistogram.javaonebusaway-gtfsrt-perf/src/main/resources/perf-data-sources.xmlonebusaway-transit-data-federation/src/test/java/org/onebusaway/transit_data_federation/impl/realtime/gtfs_realtime/GtfsRealtimeTripLibraryCharacterizationTest.javaonebusaway-transit-data-federation/src/test/resources/org/onebusaway/transit_data_federation/impl/realtime/gtfs_realtime/characterization-golden.txtpom.xml
Comment on lines
+159
to
+160
| @Override public void resetVehicleLocation(AgencyAndId vehicleId) { _delegate.resetVehicleLocation(vehicleId); } | ||
| @Override public void handleRawPosition(AgencyAndId vehicleId, double lat, double lon, long timestamp) { } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find VehicleStatusServiceImpl.handleRawPosition implementation to determine
# whether it does non-trivial work that the harness should be measuring.
rg -n -A 15 'void handleRawPosition' --type java \
onebusaway-transit-data-federation/src/main/java/org/onebusaway/transit_data_federation/impl/realtime/VehicleStatusServiceImpl.javaRepository: OneBusAway/onebusaway-application-modules
Length of output: 984
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the harness and surrounding references to understand whether raw-position
# handling is part of the intended benchmark path.
printf '\n== PerfBundleHarness excerpt ==\n'
sed -n '1,240p' onebusaway-gtfsrt-perf/src/main/java/org/onebusaway/perf/gtfsrt/PerfBundleHarness.java
printf '\n== References to handleRawPosition ==\n'
rg -n -A 3 -B 3 'handleRawPosition|setVehiclePositionsUrl|replicate the production poll path|poll path at scale' \
onebusaway-gtfsrt-perf onebusaway-transit-data-federation onebusaway-gtfs-realtime \
--type java --type mdRepository: OneBusAway/onebusaway-application-modules
Length of output: 17558
Delegate handleRawPosition to the real listener PerfBundleHarness.java:160 — GtfsRealtimeSource calls this for every vehicle position update, and VehicleStatusServiceImpl stores each raw position in its cache. Leaving it empty makes the perf harness measure less work than the real poll path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@onebusaway-gtfsrt-perf/src/main/java/org/onebusaway/perf/gtfsrt/PerfBundleHarness.java`
around lines 159 - 160, Delegate handleRawPosition in the listener adapter to
the wrapped real listener, passing through vehicleId, lat, lon, and timestamp
just like resetVehicleLocation delegates to _delegate, so
VehicleStatusServiceImpl processes every raw position update during performance
runs.
Member
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.
What & why
Investigates a production report: a 2 GB / 1 vCPU OBA box with one GTFS-RT feed (~840 trips, ~30 s poll) pegs CPU near 85% of a core. This branch builds a repeatable perf harness, profiles the poll path at King County Metro scale, and produces an evidence-backed findings report + ranked fix list. No matching-engine code is changed — the deliverable is evidence and a plan.
Headline finding
BlockConfigurationEntryImpl$BlockStopTimeList.get()constructs a newBlockStopTimeEntryImplon every.get()call (no cache —BlockConfigurationEntryImpl.java:375):It's hammered by repeated stop-time iteration on both the trip-update matching path (
applyTripUpdatesToRecord) and the block-location write path (getBlockLocation) — the latter is also the API read path, so a fix likely helps arrivals queries too. Two O(stops²) inner loops inGtfsRealtimeTripLibraryand the KCM stop-count histogram (33k trips, median 31 / max 97 stops) corroborate it.Ranked fix #1 (low cost/risk, highest reward): cache/memoize
BlockStopTimeList.get(). Full list of 4 fixes with cost/risk/reward + covering tests in the report.Honest caveat: on a desktop core one 30 s poll is ~200 ms ≈ 0.7% of a core — this repro proves the waste but does not by itself reproduce the production 85% peg (which likely also involves a slower cloud vCPU, higher poll cadence, or API read-traffic). The report recommends confirming production poll cadence + read load before attributing the exact figure.
What's in the branch
onebusaway-gtfsrt-perf— a new isolated module (allmain()classes, no tests in the default build):PerfBundleHarness,GtfsRtRefreshBenchmark(reset()+refresh() timed loop),Profiling(measure-phase async-profiler + JFR fallback),BuildKcmBundle,StopCountHistogram. Re-runGtfsRtRefreshBenchmarkto validate any fix's before/after.GtfsRealtimeTripLibraryCharacterizationTestlockscreateVehicleLocationRecordForUpdateoutput so future optimizations are provably behavior-preserving (runs in CI).docs/perf/2026-07-10-gtfsrt-matching-findings.md— the full report.docs/superpowers/.KCM GTFS/feed data, the built bundle, and flamegraphs are git-ignored (not committed).
Testing
GtfsRealtimeTripLibraryCharacterizationTest: 1/1 passing (BUILD SUCCESS).records=551every iteration (both cost stages fire), CPU + allocation flamegraphs captured.Summary by CodeRabbit
New Features
Documentation
Tests