perf(terrain): evict from the front of the point cache instead of sorting it (#1326)

Preserve oldest-fetch terrain cache eviction without sorting the full cache on each overflow.

Contribution by @daikaginza.

changelog.d/1326-terrain-cache-eviction.md

@@ -0,0 +1,3 @@
+- Evict terrain-height cache entries in fetch-time order without repeatedly
+  sorting the full cache, while keeping refreshed points newest and ordering
+  restored entries by their fetch time (daikaginza, #1326).

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<string, Promise<Array<object>>>} 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 */

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)) {

src/terrainHeightsBounds.test.mjs

@@ -184,6 +184,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';