perf: extract decoder data into concrete ephemeris and clock structs#52
Draft
giove-a wants to merge 1 commit into
Draft
perf: extract decoder data into concrete ephemeris and clock structs#52giove-a wants to merge 1 commit into
giove-a wants to merge 1 commit into
Conversation
Every navigation-data field on a decoder is Union{Nothing, T} (they fill
in as the message decodes), and the Kepler propagation and clock
correction read ~20 of them in long arithmetic chains — too many small
unions for the compiler to split, so every intermediate is boxed. That
made the ephemeris side of calc_pvt over half its runtime and roughly
70% of its allocations.
Extract the fields once per satellite per solve into concrete Float64
structs behind a function barrier: Ephemeris (the broadcast orbit plus
propagation constants, with the directly-broadcast Keplerian and
quasi-Keplerian CNAV/CNAV-2 parameterisations normalised by
orbital_terms) and ClockModel (the clock polynomial, the signal group
delay folded to a constant via the existing correct_by_group_delay
dispatch, and the Ephemeris the relativistic term is computed from).
All orbit and clock kernels now run on these, allocation-free and
bit-identical to before; the decoder-taking public API is unchanged
(thin extraction wrappers), and a decoder with a complete orbit but
not-yet-decoded clock can still be propagated.
calc_pvt on the benchmark fixtures (median, same machine):
- 9 GPS sats, warm: 18.6 µs / 21.7 KiB / 622 allocs → 11.3 µs / 14.5 KiB / 183 allocs
- 9 GPS sats, cold: 31.5 µs / 31.5 KiB / 780 allocs → 24.7 µs / 24.3 KiB / 341 allocs
- 5 Galileo sats, warm: 10.7 µs / 15.9 KiB / 422 allocs → 6.8 µs / 11.9 KiB / 179 allocs
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #52 +/- ##
==========================================
- Coverage 98.30% 96.90% -1.41%
==========================================
Files 7 8 +1
Lines 473 484 +11
==========================================
+ Hits 465 469 +4
- Misses 8 15 +7 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
Benchmark Results (Julia v1)Time benchmarks
Memory benchmarks
|
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.
Problem
Every navigation-data field on a
GNSSDecoderState'sdataisUnion{Nothing, T}(fields fill in as the message decodes). The Kepler propagation and clock correction read ~20 of them in long arithmetic chains — too many small unions for the compiler to split — so every intermediate gets boxed. Measured on the benchmark fixtures, this was over half ofcalc_pvt's runtime and roughly 70% of its allocations.Change
Extract the fields once per satellite per solve into concrete
Float64structs behind a function barrier (src/ephemeris.jl):Ephemeris— the broadcast orbit plus propagation constants. The directly-broadcast Keplerian (LNAV / I/NAV / F/NAV) and quasi-Keplerian CNAV/CNAV-2 parameterisations are normalised at extraction byorbital_terms(replacingorbital_elements), so one set of kernels serves all four navigation messages.ClockModel— the clock polynomial, the ranging-signal group delay folded to a constant via the existing (directly-tested)correct_by_group_delaydispatch, and theEphemeristhe relativistic term is computed from.All orbit and clock kernels now run on these, allocation-free and bit-identical to before. The decoder-taking public API is unchanged (thin extraction wrappers), and a decoder with a complete orbit but not-yet-decoded clock can still be propagated (the
test/cnav.jlself-consistency tests exercise exactly that, which is why orbit and clock are separate structs).Results
calc_pvton the benchmark fixtures (median, same machine):Full test suite passes; docs build cleanly (the new internals are documented on the API page).
Relation to a possible GNSSDecoder refactor
Longer-term, GNSSDecoder could publish concrete ephemeris/clock substructs itself at
validate_datapromotion time (nullable at the top, concrete inside), which would let the extraction layer here shrink to a thin adapter or disappear — see the companion GNSSDecoder issue. The kernel rewrites in this PR carry over unchanged in that scenario, so merging this now doesn't conflict with that direction.🤖 Generated with Claude Code