From 626852401a98929db4f0362420a3407253ad3aa1 Mon Sep 17 00:00:00 2001 From: daikaginza Date: Fri, 23 Oct 2026 11:57:29 +0700 Subject: [PATCH 1/2] test(terrain): pin which point the cache evicts after a refetch A stale point that is refetched becomes the newest entry, and the cache file's order is not its age order. The ceiling must still evict the oldest fetch. Passes on the current sort-based eviction. --- src/terrainHeightsBounds.test.mjs | 38 +++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/src/terrainHeightsBounds.test.mjs b/src/terrainHeightsBounds.test.mjs index 3746f4fe4..00811246d 100644 --- a/src/terrainHeightsBounds.test.mjs +++ b/src/terrainHeightsBounds.test.mjs @@ -165,6 +165,44 @@ test('the point cache stops growing, and an evicted point is simply refetched', assert.equal(asked[4], point(1)); }); +test('a refetched point counts as new when the cache evicts', async (t) => { + const root = await mkdtemp(path.join(tmpdir(), 'terrain-evict-order-')); + t.after(() => rm(root, { recursive: true, force: true })); + const cachePath = path.join(root, 'cache.json'); + const day = 24 * 3600_000; + const now = Date.now(); + const point = (i) => `-97.7${i}000,30.26000`; + const entry = (at) => ({ at, result: { ellipsoid: 150 } }); + // Written newest first: the file's order is not the age order. Point 1 is + // past the 30-day TTL, so the next request for it refetches. + await writeFile( + cachePath, + JSON.stringify({ + version: 2, + points: { + [point(3)]: entry(now - 3600_000), + [point(2)]: entry(now - day), + [point(1)]: entry(now - 31 * day), + }, + }), + ); + const asked = stubTerrainUpstream(t); + const route = install( + terrainHeightsProxy({ cacheMaxEntries: 3, cachePath }), + ).get('/api/terrain/heights'); + + await request(route, { url: `/?points=${point(1)}` }); + assert.deepEqual(asked, [point(1)], 'the stale point is refetched'); + await request(route, { url: `/?points=${point(4)}` }); + assert.equal(asked.length, 2); + + // Point 2 is now the oldest fetch; point 1 was just refreshed. + await request(route, { url: `/?points=${point(1)};${point(3)}` }); + assert.equal(asked.length, 2, 'the refreshed and the recent point stay'); + await request(route, { url: `/?points=${point(2)}` }); + assert.deepEqual(asked.slice(2), [point(2)], 'the oldest fetch was evicted'); +}); + test('an over-large cache file is not read back into memory', async (t) => { const asked = stubTerrainUpstream(t); const point = '-97.74000,30.26000'; From e60cbb42e3a310f8b8b5c71b19c0d29b92d98b7e Mon Sep 17 00:00:00 2001 From: daikaginza Date: Fri, 23 Oct 2026 11:57:37 +0700 Subject: [PATCH 2/2] perf(terrain): evict from the front of the point cache instead of sorting it Once the cache was full, every write sorted all 50,000 entries to find the oldest. A Map keeps insertion order, so keep the map in fetch-time order and delete from the front: the resolver re-inserts each point it refetches (an overwrite would leave it in its old place), and the disk load sorts once, since a file written before this change, or a migrated raw-batch cache, is not in age order. Suggested by asturksever on #975. --- server/providers/terrain.js | 21 +++++++++++++++------ src/data/terrainHeightsProxy.js | 3 +++ 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/server/providers/terrain.js b/server/providers/terrain.js index c678ab7c0..f4817f602 100644 --- a/server/providers/terrain.js +++ b/server/providers/terrain.js @@ -90,15 +90,23 @@ export function terrainHeightsProxy(options = {}) { /** * Drop the oldest entries until the map is back inside its ceiling. - * Oldest-first by fetch time, the same shape `trackBackfillProxies` uses. - * Terrain does not move, so evicting an old point costs one refetch, never - * a wrong answer. + * The map is kept in fetch-time order (sorted once on load, and the resolver + * re-inserts every point it refetches), so the oldest are at the front and + * no write sorts the map. Terrain does not move, so evicting an old point + * costs one refetch, never a wrong answer. */ function evictOverflow() { - if (mem.size <= CACHE_MAX_ENTRIES) return; + for (const key of mem.keys()) { + if (mem.size <= CACHE_MAX_ENTRIES) return; + mem.delete(key); + } + } + + /** Put the map in fetch-time order; a file is not guaranteed to be. */ + function orderByAge() { const byAge = [...mem.entries()].sort((a, b) => a[1].at - b[1].at); - for (let i = 0; i < byAge.length && mem.size > CACHE_MAX_ENTRIES; i += 1) - mem.delete(byAge[i][0]); + mem.clear(); + for (const [key, entry] of byAge) mem.set(key, entry); } /** @type {Map>>} single-flight per missing-point subset. */ const inflight = new Map(); @@ -165,6 +173,7 @@ export function terrainHeightsProxy(options = {}) { } diskDirty = mem.size > 0; } + orderByAge(); evictOverflow(); } catch { /* no disk cache yet */ diff --git a/src/data/terrainHeightsProxy.js b/src/data/terrainHeightsProxy.js index 9134f9ec6..c83b30a87 100644 --- a/src/data/terrainHeightsProxy.js +++ b/src/data/terrainHeightsProxy.js @@ -233,6 +233,9 @@ export async function resolveTerrainHeightRequest({ for (let i = 0; i < missing.length; i += 1) { const result = fetched[i]; if (validTerrainResult(result)) { + // Re-insert rather than overwrite: a Map keeps an existing key in + // its old position, and the proxy evicts from the front. + cache.delete(missing[i].key); cache.set(missing[i].key, { at: fetchedAt, result }); cacheChanged = true; } else if (absentTerrainDatum(result)) {