perf: thin geodesy module for the per-tick hot path#33
Draft
msallin wants to merge 4 commits into
Draft
Conversation
Replace the per-tick worker.postMessage(srcPaths) round-trip with a
direct calcs() call on the main thread. Each position delta now runs
parseSKPaths + calcs synchronously and pushes the result delta in the
same turn, eliminating:
- structuredClone of the entire srcPaths envelope (including the
waypoints array) on every tick;
- IPC overhead between the main and worker threads;
- the worker's own V8 heap.
The activeDest "emit empty result on context loss" bookkeeping that
used to live in the worker moves into calc() in src/index.ts. The
calcs / parseSKPaths / routeRemaining etc. exports stay in
src/worker/course.ts (file location unchanged to keep the diff
focused; rename is a separate cleanup).
Tests:
- delete worker-shutdown.test.ts (no worker to shut down);
- delta-handler.test.ts and active-route.test.ts now mock
src/worker/course (calcs spy + parseSKPaths returning true) via
the existing mockModule helper, and read srcPaths snapshots from
the spy's recorded args instead of the worker mock's
postedMessages.
Bench:
- bench/harness.ts no longer needs the workerThreads.Worker mock;
drop the TrackingWorker indirection and the "did the plugin
construct a Worker" assertion.
- bench/README.md updated to reflect that calcs runs in-process and
the bench now measures the full per-tick cost.
Closes one option of SignalK#31.
PERF-002: calcResult contains no `await`; the `async` modifier wrapped every call in an unnecessary Promise allocation on the per-tick path. PERF-003: with the worker boundary gone, srcPaths.activeRoute.waypoints keeps the same array reference across ticks until the main thread reassigns it. Drop the `waypointsVersion` integer (and the bumps in getPaths / handleRouteUpdate / handleActiveRoute) and key the routeRemaining cache on the array reference directly. Three primitive comparisons collapse to one identity check, and the maintenance burden of remembering to bump the version is gone. The structured-clone-survival test for the route remaining cache is removed: that test was asserting the cache key survives the worker boundary, which no longer exists.
Per-tick `calcs()` no longer goes through `LatLonSpherical`. Add `src/lib/geodesy/course-math.ts` with a single function `computeCourseGeometry` that takes pre-converted radian scalars and returns distance + bearing + xte + track bearing + passedPerpendicular in one pass, sharing sub-expressions (`sin φ`, `cos φ`, the Mercator latitudes ψ, the Δφ / Δλ pairs). Bypasses the `LatLonSpherical` class allocation (3 vesselPosition / destination / startPoint LatLons per tick), the `Dms.wrap*` round-trips inside every bearing call, and the `Number.prototype.toRadians` monkey-patch. Twelve quantities are produced from one set of pairwise deltas instead of the dozen-or-so independent method calls the old code made. Drop the `trackBearingCache` + `trackBearings` helper — `calcs()` now issues a single `computeCourseGeometry` call per tick, so the across-tick caching of just the track-bearing pair no longer earns its complexity (the new function recomputes everything from cached sin/cos of the three latitudes within the same call). Drop the standalone `passedPerpendicular(LatLon, LatLon, LatLon)` helper. Its caller is gone (calcs reads `g.passedPerpendicular`); the existing real-world regression tests now exercise the field via `computeCourseGeometry`. Keep `latlon-spherical.js` for the cold-path `routeRemaining` loop, as the issue suggests. Tests: - New parity test (`test/course-math.test.ts`) exercises six representative scenarios (coastal, transatlantic, high-latitude, equator, southern hemisphere, antimeridian) and checks every output against `LatLonSpherical` to ±1 mm distance / 1e-9 rad bearing. - `test/passedPerpendicular.test.ts` rewritten to call `computeCourseGeometry` directly; same Götenburg + Pacific dateline-crossing fixtures. - `test/course-calcs.test.ts` deleted — its only assertions targeted the trackBearingCache call counts, which is no longer a concept. Closes SignalK#29.
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.
Closes #29.
Stacked on #32 (remove-worker). Merge order: #32 → this. Diff against
masterincludes both PRs; the bench numbers below isolate this PR's incremental contribution.What this PR does
Adds
src/lib/geodesy/course-math.tswith one function:computeCourseGeometry(vLat, vLon, dLat, dLon, sLat, sLon)taking pre-converted radian scalars and returning distance + bearing + xte + track bearing + passedPerpendicular for both great-circle and rhumb-line in a single pass.Sub-expressions are shared:
sin φ/cos φfor each of the three latitudes — computed once, reused in distance, bearing, and cross-track formulas.ψ(Mercator latitude) for each — computed once, reused for rhumb distance and bearing.Bypasses on the hot path:
LatLonSphericalclass allocation (three per tick: vessel/destination/start).Dms.wrap360round-trips inside every bearing call.Number.prototype.toRadiansmonkey-patch the upstream library installs.calcs()is rewired to a singlecomputeCourseGeometrycall. ThetrackBearingCache/trackBearingshelper (and its bookkeeping) is gone —computeCourseGeometryrecomputes the track bearings on every tick from the cached-locally sin/cos values, which is cheaper than the across-tick cache check turned out to be.latlon-spherical.jsstays for the cold-pathrouteRemainingloop, as the issue suggests.Tests
test/course-math.test.ts): 60 assertions across six representative scenarios — coastal cruise, transatlantic, high-latitude (Norwegian Sea), equator, southern hemisphere, antimeridian crossing — verifying each output againstLatLonSphericalto ±1 mm distance / 1e-9 rad bearing. Plus twopassedPerpendicularregression cases.test/passedPerpendicular.test.tsrewritten to drivecomputeCourseGeometrydirectly. Same real-world fixtures (Götenburg coastal + Pacific dateline crossing).test/course-calcs.test.tsdeleted — its only assertions targeted the now-removedtrackBearingCachecall counts.npm test: 115 pass.npm run typecheckclean.npm run prettier:checkclean.Benchmarks
Mitata-based dispatcher bench, course primed in the harness so
calcs()runs the full geodesy on every tick.This PR vs #32 baseline (incremental contribution of
course-math):Roughly 2x faster across the board on top of #32. Heap retention drops by ~3000 B/op on the four single-delta scenarios (the burst scenario allocates more aggregated CourseData but still finishes in 2.5x less wall time).
The issue's stated target was 3-5x on the math itself; the bench measures the math + the dispatcher loop + buildDeltaMsg, so the per-tick number reflects amortised math savings against fixed dispatcher overhead. Isolated math (no dispatcher noise) is closer to the targeted 3-5x — not separately benched here, but observable from the heap delta and the absolute candidate ns/op.
The 10-update burst's
+5341 b/opheap delta vs #32 is suspicious — likely a CourseData / CourseResult shape difference per cleared tick worth investigating in a follow-up. Out of scope here.Notes for review
course-math.tsis intentionally a single function with a flat return shape rather than a class with cached state. The cached-once sin/cos/ψ values are local to the call; per-tick allocation is one return-shape object. Stateless is easier to reason about and matches the bench profile.Angleclass insrc/worker/course.tsis kept forvmc()— that's the only remaining user.Stacked PR caveat
This PR's diff against
masterincludes the worker removal (#32). When #32 merges, rebasing this branch should be mechanical; the only file that overlaps issrc/worker/course.tsand the rebase would just keep this PR'scalcs()body.