diff --git a/CHANGELOG.md b/CHANGELOG.md index 85fbc72ee..335977abb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ # Changelog +- Bound the client terrain-height cache at 20 000 entries with least-recently-used eviction, so a long session no longer retains every coordinate it ever resolved. Consumer reads promote their entry and a batch still reports every point it resolved (Pedro Lobato, #594). + - Release CCTV media streams whose upstream falls silent after answering. The 15-second media deadline covered only the wait for response headers, so a camera that replied and then stopped sending held both the proxy connection diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index c42d4428b..e00fcf8cd 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -879,6 +879,22 @@ under `gods-eye-view/server/providers/`. Portable terrain mechanics, traffic til math and GBFS source rules are available under `gods-eye-view/sources/`. The browser layers and their rendering remain in their existing modules. +## Terrain height cache bound + +The client terrain-height resolver's in-memory cache is bounded at 20 000 +entries (`DEFAULT_MAX_CACHE_ENTRIES`, overridable per instance through +`createTerrainHeights({ maxCacheEntries })`). Coordinate rounding makes repeated +visits to one place share a key but does not bound how many distinct places a +session visits, and the cache previously had no eviction, so a long session +retained every coordinate it ever resolved. Eviction is least-recently-used: +a consumer read promotes its entry, so the cell being watched is not dropped for +having been resolved early. The bound sits far above a city-scale session, so +normal use never evicts and issues no additional proxy requests. A batch's +results are assembled from what that call resolved, so eviction during a large +batch cannot report a just-resolved point as unresolved. Re:Earth and +geoid-fallback entry semantics, the fallback cooldown and the abort-time flush +are unchanged. + ## Landmark annotation identity When a landmark geocode contains only address components, annotations retain diff --git a/scripts/format-scope.json b/scripts/format-scope.json index e762d9fb6..eee546dd5 100644 --- a/scripts/format-scope.json +++ b/scripts/format-scope.json @@ -633,6 +633,7 @@ "src/data/surfaceServices.js", "src/annotations/resolver.js", "src/services/terrainHeights.js", + "src/services/terrainHeights.test.mjs", "src/services/groundFloor.js", "src/services/meshFloorSampler.js", "src/services/groundSnap.js", diff --git a/src/services/terrainHeights.js b/src/services/terrainHeights.js index 4366d632a..747eda145 100644 --- a/src/services/terrainHeights.js +++ b/src/services/terrainHeights.js @@ -1,7 +1,19 @@ import { ensureGeoidReady, geoidHeight } from '../data/geoid.js'; -/** Construct an instance-owned terrainHeights service with explicit dependencies. */ -export function createTerrainHeights({ source, signal }) { +/** Default live-entry ceiling for the in-memory cache (see CACHE_MAX_ENTRIES). */ +export const DEFAULT_MAX_CACHE_ENTRIES = 20_000; + +/** + * Construct an instance-owned terrainHeights service with explicit dependencies. + * `maxCacheEntries` overrides the cache bound; it exists so the bound is + * testable without minting twenty thousand coordinates, mirroring the injectable + * `maxBytes` on the GBFS reader. + */ +export function createTerrainHeights({ + source, + signal, + maxCacheEntries = DEFAULT_MAX_CACHE_ENTRIES, +}) { if (typeof source?.getHeights !== 'function') throw new TypeError('Terrain heights require a source'); signal?.throwIfAborted(); @@ -45,15 +57,71 @@ export function createTerrainHeights({ source, signal }) { /** Avoid repeatedly hitting a known-failing proxy from warm fallback reads. */ const GEOID_FALLBACK_COOLDOWN_MS = 60_000; + /** + * Live entry ceiling for the cache below. The rounding in point 2 of the file + * header makes repeated visits to one place share a key; it does nothing to + * bound the number of DISTINCT places a session visits, and this cache had no + * eviction, so a long session kept every coordinate it ever resolved for the + * lifetime of the page. + * + * This is a bound, not a budget. At the ~111 m floor grid a city-scale + * session stays well under it, so eviction is the pathological case rather + * than the normal one and a well-behaved session sees no extra network. Sized + * above the sibling caches (adsb.lol points 80, track history 200, TomTom + * tiles 256) because a miss here costs a proxy round trip, not just RAM. + */ + const CACHE_MAX_ENTRIES = + Number.isFinite(maxCacheEntries) && maxCacheEntries > 0 + ? Math.floor(maxCacheEntries) + : DEFAULT_MAX_CACHE_ENTRIES; + /** * In-memory cache: `"lat.toFixed(5),lon.toFixed(5)"` -> `{ellipsoid, source}`. * Module-scoped (not exported) — the only reads are through * `cachedEllipsoidalGround` and the internal lookup in - * `resolveEllipsoidalGround`. + * `resolveEllipsoidalGround`. Bounded at CACHE_MAX_ENTRIES; write through + * `cacheStore` and read a consumer-facing hit through `cacheTouch` so the + * eviction order stays least-recently-used. * @type {Map} */ const cache = new Map(); + /** + * Store an entry and drop the coldest ones once past the cap. `Map` iterates + * in insertion order, so deleting the first key evicts the least recently + * used provided every write and every consumer read re-inserts its key. + * + * Recency rather than plain insertion order because revisits here are + * spatial: a session comes back to cells near where it already was, so the + * cell it is parked on must not be evicted merely for having been resolved + * early. + * @param {string} key + * @param {{ellipsoid: number, source: 'reearth'|'geoid-fallback', retryAt?: number}} entry + */ + function cacheStore(key, entry) { + cache.delete(key); + cache.set(key, entry); + while (cache.size > CACHE_MAX_ENTRIES) { + const coldest = cache.keys().next().value; + if (coldest === undefined) break; + cache.delete(coldest); + } + } + + /** + * Read an entry and promote it to newest, so a coordinate a consumer keeps + * asking about survives eviction. Returns undefined on a miss. + * @param {string} key + * @returns {{ellipsoid: number, source: 'reearth'|'geoid-fallback', retryAt?: number}|undefined} + */ + function cacheTouch(key) { + const entry = cache.get(key); + if (entry === undefined) return undefined; + cache.delete(key); + cache.set(key, entry); + return entry; + } + /** * Builds the rounded cache key shared between the in-memory cache and the * `points` string sent to the proxy (see file header, point 2). @@ -74,7 +142,7 @@ export function createTerrainHeights({ source, signal }) { * @returns {number|null} */ function cachedEllipsoidalGround(lat, lon) { - const entry = cache.get(cacheKey(lat, lon)); + const entry = cacheTouch(cacheKey(lat, lon)); return entry ? entry.ellipsoid : null; } @@ -91,7 +159,7 @@ export function createTerrainHeights({ source, signal }) { * @returns {number|null} */ function cachedRealEllipsoidalGround(lat, lon) { - const entry = cache.get(cacheKey(lat, lon)); + const entry = cacheTouch(cacheKey(lat, lon)); return entry && entry.source === 'reearth' ? entry.ellipsoid : null; } @@ -213,6 +281,12 @@ export function createTerrainHeights({ source, signal }) { uncached.push(item); } + // What this call resolved, so the assembly below can report a point even + // if a later chunk's writes pushed it past CACHE_MAX_ENTRIES. Without it a + // batch big enough to evict its own earlier chunks would report points as + // unresolved that it had in fact just resolved. + const resolvedThisCall = new Map(); + // Resolve the network path in sequential <=CHUNK_SIZE chunks. Each chunk's // failure is isolated to that chunk's points (geoid fallback), so a single // bad chunk doesn't lose results for the rest of a large batch. @@ -230,7 +304,9 @@ export function createTerrainHeights({ source, signal }) { // one contact frozen at the geoid while its neighbors resolved). // An omitted point now caches nothing and retries on the next warm. if (Number.isFinite(ellipsoid)) { - cache.set(item.key, { ellipsoid, source: 'reearth' }); + const entry = { ellipsoid, source: 'reearth' }; + resolvedThisCall.set(item.key, entry); + cacheStore(item.key, entry); } } } catch { @@ -247,11 +323,13 @@ export function createTerrainHeights({ source, signal }) { item.lon, item.sourceOrthometricM, ); - cache.set(item.key, { + const entry = { ellipsoid, source: 'geoid-fallback', retryAt: Date.now() + GEOID_FALLBACK_COOLDOWN_MS, - }); + }; + resolvedThisCall.set(item.key, entry); + cacheStore(item.key, entry); } } } @@ -260,7 +338,7 @@ export function createTerrainHeights({ source, signal }) { // the upstream omitted has NO entry (round 6 — deliberately uncached so it // retries later): report it unresolved instead of throwing. return work.map((item) => { - const entry = cache.get(item.key); + const entry = cache.get(item.key) || resolvedThisCall.get(item.key); return entry ? { ellipsoid: entry.ellipsoid, source: entry.source } : { ellipsoid: null, source: 'unresolved' }; diff --git a/src/services/terrainHeights.test.mjs b/src/services/terrainHeights.test.mjs new file mode 100644 index 000000000..c9884e89c --- /dev/null +++ b/src/services/terrainHeights.test.mjs @@ -0,0 +1,130 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { + createTerrainHeights, + DEFAULT_MAX_CACHE_ENTRIES, +} from './terrainHeights.js'; + +/** + * A terrain source that always resolves, so these tests exercise the cache and + * never the geoid-fallback path. Heights are derived from the coordinate so an + * assertion can name the value it expects. + */ +function stubSource() { + const batches = []; + return { + batches, + async getHeights(chunk) { + batches.push(chunk.map((item) => item.key)); + return chunk.map((item) => ({ ellipsoid: heightAt(item.lon) })); + }, + }; +} + +/** Distinct 5-decimal coordinates along the equator: 0.001, 0.002, … */ +const lonAt = (i) => i / 1000; +const heightAt = (lon) => 1000 + Math.round(lon * 1000); +const pointAt = (i) => ({ lat: 0, lon: lonAt(i) }); + +test('the default cache bound is exported so callers can reason about it', () => { + assert.equal(DEFAULT_MAX_CACHE_ENTRIES, 20_000); +}); + +test('the in-memory cache stops growing once it reaches its entry bound', async () => { + const terrain = createTerrainHeights({ + source: stubSource(), + maxCacheEntries: 4, + }); + + // Six distinct coordinates through a cache that holds four. + for (let i = 0; i < 6; i += 1) { + await terrain.resolveEllipsoidalGround([pointAt(i)]); + } + + // The two coldest are gone. Before the bound existed the cache kept every + // coordinate a session ever resolved, so both of these still answered. + assert.equal(terrain.cachedEllipsoidalGround(0, lonAt(0)), null); + assert.equal(terrain.cachedEllipsoidalGround(0, lonAt(1)), null); + + // The four most recent are untouched — a bound, not a flush. + for (let i = 2; i < 6; i += 1) { + assert.equal( + terrain.cachedEllipsoidalGround(0, lonAt(i)), + heightAt(lonAt(i)), + `coordinate ${i} stayed warm`, + ); + } +}); + +test('a coordinate a consumer keeps reading outlives an equally old one it ignores', async () => { + const terrain = createTerrainHeights({ + source: stubSource(), + maxCacheEntries: 4, + }); + for (let i = 0; i < 4; i += 1) { + await terrain.resolveEllipsoidalGround([pointAt(i)]); + } + + // Read coordinate 0 and ignore coordinate 1. They are otherwise the same age, + // so insertion order alone could not tell them apart. + assert.equal( + terrain.cachedEllipsoidalGround(0, lonAt(0)), + heightAt(lonAt(0)), + ); + + // Two fresh coordinates push the cache two entries past its bound. + for (let i = 4; i < 6; i += 1) { + await terrain.resolveEllipsoidalGround([pointAt(i)]); + } + + assert.equal( + terrain.cachedEllipsoidalGround(0, lonAt(0)), + heightAt(lonAt(0)), + 'the cell being read survived eviction', + ); + assert.equal( + terrain.cachedEllipsoidalGround(0, lonAt(1)), + null, + 'the same-age cell nobody read was evicted instead', + ); +}); + +test('a batch larger than the cache bound still reports every point it resolved', async () => { + const terrain = createTerrainHeights({ + source: stubSource(), + maxCacheEntries: 4, + }); + const batch = Array.from({ length: 10 }, (_, i) => pointAt(i)); + + const results = await terrain.resolveEllipsoidalGround(batch); + + // Eviction inside the call must not turn a point this batch resolved into an + // unresolved one: the results are assembled from what the call produced, not + // from whatever survived in the cache by the time it finished. + assert.equal(results.length, batch.length); + results.forEach((result, i) => { + assert.equal(result.source, 'reearth', `point ${i} reported resolved`); + assert.equal(result.ellipsoid, heightAt(lonAt(i))); + }); + + // The cache still honours the bound afterwards. + const warm = batch.filter( + (point) => terrain.cachedEllipsoidalGround(point.lat, point.lon) !== null, + ); + assert.equal(warm.length, 4); +}); + +test('an out-of-range cache bound falls back to the default instead of disabling the cache', async () => { + for (const maxCacheEntries of [0, -1, Number.NaN, undefined]) { + const terrain = createTerrainHeights({ + source: stubSource(), + maxCacheEntries, + }); + await terrain.resolveEllipsoidalGround([pointAt(1)]); + assert.equal( + terrain.cachedEllipsoidalGround(0, lonAt(1)), + heightAt(lonAt(1)), + `maxCacheEntries=${String(maxCacheEntries)} kept a working cache`, + ); + } +});