diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b71b6b09..0148ef5f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,8 @@ - Director timeline seeks cross the dateline along the short longitude arc. +- Frequently reused terrain batches retain their cached heights ahead of unused cells. + - Recent Imagery’s exported `setBox` and `searchHls` reject missing, blank or boolean programmatic box edges instead of interpreting them as zero coordinates. diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index 7f9c94ff4..56f4dbb38 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -1198,11 +1198,11 @@ entries (`DEFAULT_MAX_CACHE_ENTRIES`, overridable per instance through 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 +a consumer read, including a warm hit in a batch lookup, 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 +results are assembled from what that call resolved or found warm, so eviction +during a large batch cannot report one of its points as unresolved. Re:Earth and geoid-fallback entry semantics, the fallback cooldown and the abort-time flush are unchanged. diff --git a/src/services/terrainHeights.js b/src/services/terrainHeights.js index a6502aeb5..f84a4413f 100644 --- a/src/services/terrainHeights.js +++ b/src/services/terrainHeights.js @@ -272,28 +272,26 @@ export function createTerrainHeights({ const uncached = []; const seenKeys = new Set(); const now = Date.now(); + // What this call resolved or found warm, 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 entries would report + // points as unresolved that it had in fact resolved. + const resolvedThisCall = new Map(); for (const item of work) { - const entry = cache.get(item.key); + const entry = cacheTouch(item.key); const fallbackCooling = entry?.source === 'geoid-fallback' && Number.isFinite(entry.retryAt) && now < entry.retryAt; - if ( - (entry && entry.source === 'reearth') || - fallbackCooling || - seenKeys.has(item.key) - ) + if ((entry && entry.source === 'reearth') || fallbackCooling) { + resolvedThisCall.set(item.key, entry); continue; + } + if (seenKeys.has(item.key)) continue; seenKeys.add(item.key); 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. diff --git a/src/services/terrainHeights.test.mjs b/src/services/terrainHeights.test.mjs index c9884e89c..5e80b9be2 100644 --- a/src/services/terrainHeights.test.mjs +++ b/src/services/terrainHeights.test.mjs @@ -114,6 +114,47 @@ test('a batch larger than the cache bound still reports every point it resolved' assert.equal(warm.length, 4); }); +test('warm batch consumers retain their terrain cells ahead of unused cells', async () => { + let calls = 0; + const terrain = createTerrainHeights({ + maxCacheEntries: 2, + source: { + async getHeights(points) { + calls++; + return points.map((p) => ({ ellipsoid: 100 + p.lon })); + }, + }, + }); + const p = (lon) => ({ lat: 0, lon }); + await terrain.resolveEllipsoidalGround([p(1)]); + await terrain.resolveEllipsoidalGround([p(2)]); + await terrain.resolveEllipsoidalGround([p(1)]); + assert.equal(calls, 2); + await terrain.resolveEllipsoidalGround([p(3)]); + assert.equal(terrain.cachedEllipsoidalGround(0, 1), 101); + assert.equal(terrain.cachedEllipsoidalGround(0, 2), null); + assert.equal(calls, 3); +}); + +test('a batch still reports its warm points after its own fetches evict them', async () => { + const terrain = createTerrainHeights({ + source: stubSource(), + maxCacheEntries: 2, + }); + await terrain.resolveEllipsoidalGround([pointAt(0)]); + + const results = await terrain.resolveEllipsoidalGround([ + pointAt(0), + pointAt(1), + pointAt(2), + ]); + + results.forEach((result, i) => { + assert.equal(result.source, 'reearth', `point ${i} reported resolved`); + assert.equal(result.ellipsoid, heightAt(lonAt(i))); + }); +}); + 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({