From 1fae10dc31ee139e7da8e70cbddc51c307c20f16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kasper=20H=C3=A4gele?= Date: Wed, 30 Sep 2026 10:34:31 +0200 Subject: [PATCH] perf(app): hand a map collection to MapLibre only when it changed The 1 Hz tick called setData on every source, changed or not, and MapLibre serialises, tiles and uploads whatever it is handed. On a Nijmegen-to-Arnhem export (4559 receptions, 4478 rays) a tick with nothing new took 96.6 ms of main thread on a laptop, and 11.8 ms with this change. put() remembers what each source was last handed (lastSent) and skips a collection that is the same object. The dots, the pillars, the hex and the noise cells already come out of a cache that returns the same object on a hit. The rays are built in about a millisecond from five inputs, so their answer is compared instead (sameFeatures) and an equal one reuses last draw's object. The 3D rays read the terrain height when their buffers are filled, so with the mesh on they are still filled on every draw. Co-Authored-By: Claude Fable 5.1 --- app/src/__tests__/huntmap-contract.test.js | 35 ++++++++ app/src/__tests__/rendercache.test.js | 97 +++++++++++++++++++++- app/src/huntmap.js | 47 +++++++++-- app/src/rendercache.js | 56 +++++++++++++ 4 files changed, 225 insertions(+), 10 deletions(-) diff --git a/app/src/__tests__/huntmap-contract.test.js b/app/src/__tests__/huntmap-contract.test.js index 00dd7b3b..14f9211c 100644 --- a/app/src/__tests__/huntmap-contract.test.js +++ b/app/src/__tests__/huntmap-contract.test.js @@ -56,3 +56,38 @@ describe('applyLayerVisibility covers every layer layerVisibility decides', () = expect([...applied].sort()).toEqual([...decided].sort()) }) }) + +// The 1 Hz tick hands every collection to MapLibre, and MapLibre tiles and +// uploads whatever it is handed, changed or not: 93 ms for 4501 rays and 60 ms +// for as many dots on a laptop, once a second. The collections below come out +// of a cache or a comparison, so an unchanged one is the same object, and +// put() leaves it where it is (lastSent, rendercache.js). That only holds while +// nothing writes those sources around it, and a direct setData reads like the +// lines next to it, so the next edit to draw() is where it would come back. +describe('the collections a tick can leave unchanged go to the map through put()', () => { + const guarded = ['noise', 'points', 'points-3d', 'reach'] + + it('finds put() and its guard at all', () => { + expect(mapSrc).toMatch(/const put = \(id, data\) => \{ if \(sent\.isNew\(id, data\)\) map\.getSource\(id\)\.setData\(data\) \}/) + }) + + it.each(guarded)('%s is never written directly', (id) => { + expect(mapSrc).not.toContain(`getSource('${id}').setData(`) + expect(mapSrc).toContain(`put('${id}',`) + }) + + // The two hex layers (#634) are written in one loop over their slots. + it('writes both hex layers through put()', () => { + const cells = mapSrc.slice(mapSrc.indexOf('function drawCells'), mapSrc.indexOf('function applyFades')) + expect(cells).toContain("for (const [id, res] of [['hex', slots.a], ['hex-b', slots.b]])") + expect(cells).toContain('put(id, ') + expect(cells).not.toContain('.setData(') + expect(mapSrc).not.toContain("getSource('hex-b').setData(") + }) + + it('forgets what it sent when the overlays are mounted again, since the sources come back empty', () => { + const mount = mapSrc.slice(mapSrc.indexOf('function addOverlays'), mapSrc.indexOf('function applyBasemap')) + expect(mount.length).toBeGreaterThan(0) + expect(mount).toContain('sent.clear()') + }) +}) diff --git a/app/src/__tests__/rendercache.test.js b/app/src/__tests__/rendercache.test.js index 25c2c6ae..af424013 100644 --- a/app/src/__tests__/rendercache.test.js +++ b/app/src/__tests__/rendercache.test.js @@ -1,5 +1,5 @@ import { describe, it, expect, vi } from 'vitest' -import { recordsKey, lastValueCache, hueKey, selectionKey, ownersKey, rowCache } from '../rendercache.js' +import { recordsKey, lastValueCache, hueKey, selectionKey, ownersKey, rowCache, sameFeatures, lastSent } from '../rendercache.js' const rec = (id) => ({ id, lat: 51, lon: 4, rssi: -70 }) @@ -268,3 +268,98 @@ describe('rowCache', () => { expect(compute).toHaveBeenCalledTimes(3) }) }) + +describe('sameFeatures', () => { + // What coverageFeatures emits per hearing: a hub-to-hearing line and the + // ray's look. Built fresh on every call, as every tick builds them. + const ray = (over = {}) => ({ type: 'Feature', + geometry: { type: 'LineString', coordinates: [[5.85, 51.84], [5.86, 51.85]] }, + properties: { id: 'ab12', color: '#4f8cff', op: 0.62, w: 1.8, two: false, dim: false, alt: 30, hub: 'advertised', ...over } }) + + it('holds across the fresh objects every tick builds', () => { + expect(sameFeatures([ray(), ray({ id: 'cd34' })], [ray(), ray({ id: 'cd34' })])).toBe(true) + }) + + it('sees a ray arrive', () => { + expect(sameFeatures([ray()], [ray(), ray()])).toBe(false) + }) + + it('sees a hub move, every property the same', () => { + const moved = ray() + moved.geometry.coordinates[0][1] = 51.8401 + expect(sameFeatures([ray()], [moved])).toBe(false) + }) + + it('sees a selection dim a ray', () => { + expect(sameFeatures([ray()], [ray({ dim: true, op: 0.155 })])).toBe(false) + }) + + it('sees a property it was never told about', () => { + // Generic over the properties on purpose: a field added to the rays later + // must not be one this comparison forgets. + expect(sameFeatures([ray()], [ray({ added: 1 })])).toBe(false) + expect(sameFeatures([ray({ added: 1 })], [ray({ added: 2 })])).toBe(false) + expect(sameFeatures([ray({ added: 1 })], [ray()])).toBe(false) + }) + + it('sees two rays swap places, since the later one paints on top', () => { + const a = ray({ id: 'ab12' }), b = ray({ id: 'cd34' }) + expect(sameFeatures([a, b], [b, a])).toBe(false) + }) + + it('sees a geometry of another kind or length', () => { + const points = ray() + points.geometry.type = 'MultiPoint' // the same coordinates, drawn as two dots + expect(sameFeatures([ray()], [points])).toBe(false) + const longer = ray() + longer.geometry.coordinates.push([5.87, 51.86]) + expect(sameFeatures([ray()], [longer])).toBe(false) + }) + + it('calls two empty lists the same, and anything that is not a list different', () => { + expect(sameFeatures([], [])).toBe(true) + expect(sameFeatures(null, [])).toBe(false) + expect(sameFeatures([ray()], undefined)).toBe(false) + }) +}) + +describe('lastSent', () => { + it('lets a collection through once, and not again while it is the same object', () => { + const sent = lastSent() + const fc = { type: 'FeatureCollection', features: [] } + expect(sent.isNew('points', fc)).toBe(true) + expect(sent.isNew('points', fc)).toBe(false) + expect(sent.isNew('points', fc)).toBe(false) + }) + + it('goes by the object, not by what is in it', () => { + // The caches hand back the same object on a hit, so identity is the whole + // test; comparing contents here would cost what the skip is meant to save. + const sent = lastSent() + expect(sent.isNew('points', { type: 'FeatureCollection', features: [] })).toBe(true) + expect(sent.isNew('points', { type: 'FeatureCollection', features: [] })).toBe(true) + }) + + it('keeps each source apart', () => { + const sent = lastSent() + const empty = { type: 'FeatureCollection', features: [] } + expect(sent.isNew('points', empty)).toBe(true) + expect(sent.isNew('hex', empty)).toBe(true) + expect(sent.isNew('points', empty)).toBe(false) + }) + + it('remembers only the last one, so a collection that comes back is sent again', () => { + const sent = lastSent() + const a = { features: [] }, b = { features: [] } + sent.isNew('points', a); sent.isNew('points', b) + expect(sent.isNew('points', a)).toBe(true) + }) + + it('forgets everything on clear(), which is what a style swap needs: the sources come back empty', () => { + const sent = lastSent() + const fc = { features: [] } + sent.isNew('points', fc) + sent.clear() + expect(sent.isNew('points', fc)).toBe(true) + }) +}) diff --git a/app/src/huntmap.js b/app/src/huntmap.js index 664a436f..2fc79d48 100644 --- a/app/src/huntmap.js +++ b/app/src/huntmap.js @@ -11,7 +11,7 @@ import { layerVisibility, pitchTransition } from './maplayers.js' import { coverageStars, coverageFeatures, assignHues, selectionDim, starKeyOf, starSelected } from './coverage.js' import { createRayLayer } from './raylayer.js' import { octagonRing, pillarRadiusM, collapsePillars, PILLAR_MERGE_M } from './pointmarker.js' -import { recordsKey, lastValueCache, hueKey, selectionKey, ownersKey } from './rendercache.js' +import { recordsKey, lastValueCache, hueKey, selectionKey, ownersKey, sameFeatures, lastSent } from './rendercache.js' import { currentRideStart, isBacklog } from './rides.js' import { hexCellLabel, planHexLabels } from './hexlabels.js' import { senderText } from './receptionlog.js' @@ -176,6 +176,15 @@ export function createHuntMap(containerId) { // starCache keeps each star's estimate from tick to tick (coverage.js). const coverageSel = new Set(), starCache = new Map() let coverageHue = new Map(), lastReachRows = null, hubFeatures = [] + // What each source was last handed. The tick draws once a second, and a + // collection that did not change since then stays where it is: put() for a + // GeoJSON source, and the same memory for the 3D rays' buffers. + const sent = lastSent() + const put = (id, data) => { if (sent.isNew(id, data)) map.getSource(id).setData(data) } + // The rays of the last draw, kept so a tick that builds the same ones hands + // over the same object (sameFeatures), and whether the mesh was on when the + // 3D buffers were last filled. + let lastRays = EMPTY, raysOnMesh = false // A reception's attribution by reach (#661, attribution.js): app.js works it // out every tick and puts it on the row as _attr, so the stars, the dots and // the node-position layer place a relay id exactly as the HUD names it. @@ -184,6 +193,16 @@ export function createHuntMap(containerId) { toMerc: (lon, lat, alt) => maplibregl.MercatorCoordinate.fromLngLat([lon, lat], alt), elevation: (lon, lat) => (typeof map.queryTerrainElevation === 'function' && map.getTerrain && map.getTerrain() ? (map.queryTerrainElevation([lon, lat]) || 0) : 0), }) + // The 3D rays read the terrain height under both ends when their buffers + // are filled, and the mesh's tiles arrive after it is switched on. So with + // the mesh on they are filled on every draw, as before, and once more on the + // first draw after it went off; otherwise only when the rays changed. + function putRays3D(fc) { + const mesh = !!(map.getTerrain && map.getTerrain()) + const changed = sent.isNew('reach-3d', fc) + if (changed || mesh || raysOnMesh) rays.setData(fc.features) + raysOnMesh = mesh + } const coverageOn = () => nodeLayerMode === 'reach' function applyReachVisibility() { if (map.getLayer('reach')) map.setLayoutProperty('reach', 'visibility', coverageOn() && !mode3D ? 'visible' : 'none') @@ -231,7 +250,7 @@ export function createHuntMap(containerId) { hubFeatures = [] if (!coverageOn() || !map.getSource('reach')) { coverageHue = new Map(); starCache.clear() - if (map.getSource('reach')) { map.getSource('reach').setData(EMPTY); rays.setData([]) } + if (map.getSource('reach')) { lastRays = EMPTY; put('reach', EMPTY); putRays3D(EMPTY) } return null } const byKey = new Map(nodePositions.map((n) => [String(n.pubkey).toLowerCase(), n])) @@ -243,11 +262,16 @@ export function createHuntMap(containerId) { const hues = assignHues(stars.map((st) => ({ id: st.id, lat: st.origin.lat, lon: st.origin.lon }))) const colorOf = (slot) => cssVar(`--ch-hue-${slot}`) const selected = coverageSelected() - const fcRays = coverageFeatures(stars, { slotOf: (id) => hues.get(id), colorOf, selected }) + let fcRays = coverageFeatures(stars, { slotOf: (id) => hues.get(id), colorOf, selected }) + // Built in about a millisecond and dear to hand over, so the answer is + // compared rather than its inputs signed: the same rays as last draw are + // last draw's object, which put() and putRays3D() leave where it is. + if (sameFeatures(fcRays.features, lastRays.features)) fcRays = lastRays + else lastRays = fcRays const selectedStars = new Set(selected.size ? stars.filter((st) => starSelected(st, selected)).map((st) => st.id) : []) coverageHue = new Map([...hues].map(([id, slot]) => [id, colorOf(slot)])) - map.getSource('reach').setData(fcRays) - rays.setData(fcRays.features) + put('reach', fcRays) + putRays3D(fcRays) // The ● hub of a star with no registry position, in the star's hue. A // feature of the dot layer since #632, drawn with the estimates by // drawNodeLayer, so one setData carries every ●. A tap selects the star. @@ -791,6 +815,9 @@ export function createHuntMap(containerId) { // the rows say: a theme swap or the bare fallback used to leave the ▲ and // ● gone until something else changed (#632 review). nodePosSig = null + // The same for every source put() writes and for the rays' buffers: a + // style swap brought them back empty, so nothing counts as sent. + sent.clear(); raysOnMesh = false draw() } // Initial style: 'load' fires once when the first style is ready. A theme @@ -899,8 +926,10 @@ export function createHuntMap(containerId) { // the points wait for their zoom, and in 3D that is what keeps a pillar // per reception off the GPU until the closest one (#634). const pointsOn = pointShare(zoom, view) > 0 - map.getSource('points').setData(vis.points && pointsOn ? buildPointsFC(records, sel, owners) : EMPTY) - map.getSource('points-3d').setData(vis['points-3d'] && pointsOn ? buildPoints3DFC(records, sel, owners) : EMPTY) + // put(): each of these is the same object as last tick when its cache + // hit, and EMPTY is one constant, so only what changed is handed over. + put('points', vis.points && pointsOn ? buildPointsFC(records, sel, owners) : EMPTY) + put('points-3d', vis['points-3d'] && pointsOn ? buildPoints3DFC(records, sel, owners) : EMPTY) map.getSource('trail').setData(buildTrailFC()) // The trail belongs to no repeater, so it is never part of a selection and // always steps back with one. One LineString with no properties, so this @@ -923,10 +952,10 @@ export function createHuntMap(containerId) { const slots = hexSlots(map.getZoom(), HEX_MAX_RES) const hexOn = vis.hex || vis['hex-3d'] for (const [id, res] of [['hex', slots.a], ['hex-b', slots.b]]) { - map.getSource(id).setData(hexOn && res != null ? buildHexFC(records, cellsFor.sel, cellsFor.owners, res, id) : EMPTY) + put(id, hexOn && res != null ? buildHexFC(records, cellsFor.sel, cellsFor.owners, res, id) : EMPTY) } applyFades(slots) - map.getSource('noise').setData(vis.noise ? buildNoiseFC() : EMPTY) + put('noise', vis.noise ? buildNoiseFC() : EMPTY) drawHexLabels(records, vis['hex-labels']) zoomDrawn.cells = zoomKeysNow().cells } diff --git a/app/src/rendercache.js b/app/src/rendercache.js index f0b0ad29..43f24c1c 100644 --- a/app/src/rendercache.js +++ b/app/src/rendercache.js @@ -175,3 +175,59 @@ export function rowCache() { }, } } + +// sameFeatures says whether two feature lists would draw the same: the same +// geometries and the same properties, in the same order. For a collection that +// is cheap to build and dear to hand over, where signing the inputs would cost +// more care than building: the rays (#603) come out of the records, the +// registry, the attribution, the selection and the theme, and are built in +// about a millisecond. So the answer is compared instead, and a collection +// equal to the last one is not sent again (lastSent). +// +// Generic over the properties on purpose: a field added to the features later +// is compared without anyone remembering to add it here. Exact, not a fold, so +// there is no collision that would keep last tick's map up. +export function sameFeatures(a, b) { + if (!Array.isArray(a) || !Array.isArray(b) || a.length !== b.length) return false + for (let i = 0; i < a.length; i++) if (!sameFeature(a[i], b[i])) return false + return true +} +function sameFeature(a, b) { + if (a === b) return true + if (!a || !b) return false + const ga = a.geometry, gb = b.geometry + if (!ga || !gb || ga.type !== gb.type || !sameCoords(ga.coordinates, gb.coordinates)) return false + const pa = a.properties || {}, pb = b.properties || {} + const keys = Object.keys(pa) + if (keys.length !== Object.keys(pb).length) return false + for (const k of keys) if (pa[k] !== pb[k] || !(k in pb)) return false + return true +} +function sameCoords(a, b) { + if (!Array.isArray(a) || !Array.isArray(b)) return a === b + if (a.length !== b.length) return false + for (let i = 0; i < a.length; i++) if (!sameCoords(a[i], b[i])) return false + return true +} + +// lastSent remembers, per source, the collection the map was last handed, so +// one that did not change is not handed over again. MapLibre does not look: +// setData serialises the collection, ships it to the worker, tiles it again +// and uploads the result whether or not it is the object it already has. For +// the 4501 rays of a Nijmegen-to-Arnhem export that was 93 ms per call on a +// laptop, and the map drew once a second. +// +// By identity, which is what makes it free: the caches above hand back the +// same object on a hit. clear() is for a style swap, after which every source +// exists again and is empty. +export function lastSent() { + const sent = new Map() + return { + isNew(id, data) { + if (sent.has(id) && sent.get(id) === data) return false + sent.set(id, data) + return true + }, + clear() { sent.clear() }, + } +}