Performance improvements refactor #11

Open
opened 2026-07-30 12:31:36 +00:00 by jeroen · 6 comments
Owner

Problem

The client has nine independent data loaders (map tile textures, vector surface geometry, the globe quadtree, 3DBAG LoD2.2, OSM/Overpass buildings, POI data layers, routing, transit vehicles, weather). They were migrated onto the ADR-033 seams (platform::http, platform::storage, platform::work::Dispatcher, platform::workers) one at a time, and the migration is uneven. Five concrete problems fall out of that, all citable:

1. The same source tile is fetched, read and re-parsed many times over

There is no in-memory cache and no single-flight anywhere in the tile path. Every consumer calls tile_loader::load_tile_image (crates/viberfox/src/systems/tile_loader.rs:169), which checks the on-disk blob cache and otherwise hits the network. Two consumers asking for the same tile in the same frame both miss and both fetch, because each streamer de-dups only against its own requested set.

The overzoom fan-out makes this large. A fresh anchor opens at DEFAULT_ANCHOR_ZOOM = 17 (map_stream.rs:248) and update_map_stream builds a clipmap of MIN_ZOOM..=current_zoom = z10–z17 (map_stream.rs:59, :424), radius LOAD_RADIUS = 4 at the finest zoom and COARSE_RADIUS = 3 below (:53, :56, :429) — 9×9 + 7×(7×7) = 424 tile requests. Shortbread tops out at MAX_SOURCE_ZOOM = 14 (vector_tiles.rs:40), so the 179 requests at z15/z16/z17 are all resolved by SubTile::resolve to a z14 ancestor (vector_tiles.rs:67, reached via tile_loader.rs:240). Those 179 requests resolve to only a few dozen distinct z14 bodies — and the z14 ring (49 tiles, radius 3 around the same camera) requests essentially all of them in the same frame anyway.

Each of those 179 requests independently:

  • reads the ~500 KB body (a std::fs::read per request natively — platform/storage.rs:56);
  • re-fetches it over the network on the web build, because the wasm storage impl is still a None-returning stub (platform/storage.rs:83-90);
  • re-runs Reader::new(bytes.to_vec()) — a full copy plus a prost decode of the whole tile (vector_tiles.rs:250).

Then the same z14 bodies are fetched again by the other consumers:

  • map_geometry streams 3×3 cells at the same GEOMETRY_ZOOM = MAX_SOURCE_ZOOM (map_geometry.rs:51, :55) and parses each body four more timestessellate_surfaces (:136), tessellate_roads (:142), scatter_vegetation (:153) and water_shore_field (:166), each constructing its own Reader (vector_tiles.rs:725, :843, vegetation.rs:142, vector_tiles.rs:1278);
  • routing fetches up to MAX_ROUTE_TILES = 96 z14 tiles per solve (routing.rs:69, navigation.rs:485);
  • load_globe_tile_rgba parses each globe tile twice — once in render_mvt, once in ocean_mask (tile_loader.rs:302, :305).

Per docs/notes/tile-raster-cost.md (measured 2026-07-25), MVT parse is ~2.7 ms and get_features ~4 ms on the Groningen z14 fixture, against ~121 ms to rasterise. So the duplicated parse is not the headline cost — but the duplicated fetch is unbounded bandwidth on web, and the duplicated disk read is 179 × 500 KB per anchor natively.

Smaller, in the same family: render_mvt walks PAINT_ORDER, which lists streets twice (vector_tiles.rs:181-182, casing then fill) and buildings twice (:190, :191), and calls reader.get_features(idx) inside that loop (:278). mvt_reader::Reader::get_features decodes geometry and properties into a fresh Vec<Feature> on every call (mvt-reader-2.4.0/src/lib.rs:208), so the two heaviest layers of every tile are decoded twice per render.

2. Half the loaders still do their CPU work on the wasm main thread

docs/notes/wasm-performance.md records the fix: on wasm Bevy disables multi_threaded, so IoTaskPool is the main thread, and parse/mesh work must be handed to platform::workers (platform/workers.rs:9-25). That was done for map_stream (:308), map_geometry (:133), lod22 (:228) and osm_buildings (:543, :640). It was not done for:

Loader CPU work still inside the IoTaskPool future
globe.rs:870-888 load_globe_tile_rgba (MVT rasterise + ocean_mask, or a PNG decode + paint_water_alpha_by_colour) and decode_globe_tile's mip chain — up to QT_FETCH_BUDGET = 24 in flight (globe.rs:104)
tile_loader.rs:60-83 load_tile_rgba + build_mip_chain
data_layers.rs:530-537 build_layer_cellparse_pois (serde_json::Value, :1336) + the merged marker mesh (:1273-1308)
transit_vehicles.rs:318 parse_vehicle_positions (GTFS-RT protobuf) + parse_line_map (serde_json::Value, :566) every POLL_INTERVAL
navigation.rs:445-452 the whole solve: RoadGraph::add_tile per tile (its own Reader::new, routing.rs:424) plus graph.finish()'s segment splitting

navigation::solve also fetches its corridor tiles serially — a for loop with an .await per tile (navigation.rs:483-501) — so a 96-tile route pays 96 sequential round trips even natively.

3. map_stream is the one streamer with no concurrency bound

Every other streamer gates admission: tile_loader Dispatcher::new(TILE_WORKERS = 3, …) (tile_loader.rs:15, :46), lod22 LOD22_CONCURRENCY = 3 (lod22.rs:44, :156), data_layers serial + Overpass-throttled (data_layers.rs:464), osm_buildings serial (osm_buildings.rs:464), map_geometry SURFACE_MAX_IN_FLIGHT = 8 (map_geometry.rs:82), globe QT_FETCH_BUDGET = 24 (globe.rs:1238). map_stream spawns one detached task per newly-wanted tile with no cap at all (map_stream.rs:486-495), so a fresh anchor or a re-anchor releases all 424 at once. platform::work::Dispatcher exists for exactly this (platform/work.rs:18) and map_stream is the only tile streamer that does not use it.

4. Six streamers hand-roll the same skeleton

The same five-or-six fields, with the same semantics and slightly different bugs, appear in:

requested set failed + backoff results sink in-flight bound despawn queue + per-frame cap anchor generation
map_stream.rs:130-145 (u32, Instant) Arc<Mutex<HashMap>> ✓ 12/frame
map_geometry.rs:96-112 Arc<Mutex<Vec>> ✓ counter ✓ 4/frame
globe.rs:165-184 pending (u32, Instant) Arc<Mutex<Vec>> ✓ budget ✓ 8/frame
lod22.rs:99-114 u32 Arc<Mutex<Vec>> Dispatcher ✗ — all at once (:138-140)
data_layers.rs:266-282, :328-329 u32 + retry_at Arc<Mutex<Vec>> Dispatcher ✗ — all at once (:441-451)
osm_buildings.rs:250-278 far_requested u32 Arc<Mutex<Vec>> Dispatcher

The two ✗ cells are the interesting ones: CLAUDE.md's known-issues section names mass despawn in one command buffer as the VK_ERROR_DEVICE_LOST pattern, and four streamers have a per-frame despawn cap for that reason. lod22's re-anchor path despawns every cell in one frame (lod22.rs:138-140, up to (2·LOD22_RADIUS+1)² = 25 merged 3DBAG meshes), and data_layers' re-anchor despawns every marker entity of every enabled layer in one frame (data_layers.rs:441-451).

5. Per-frame waste in update_map_stream

The 424-entry wanted: HashMap<TileKey, f32> and the 424-entry spawn_order: Vec<TileKey> are rebuilt from scratch every frame, unconditionally (map_stream.rs:419-450), as is the stale scan over loaded (:531-536). The desired set only changes when the camera crosses a tile boundary or the LOD zoom steps. TileKey is 20 bytes of plain data but is Clone-not-Copy (crates/vibe_core/src/world.rs:6), so each rebuild is also ~850 TileKey clones.

Approach

Five steps, each independently compilable and testable. Order matters: step 1 is what the rest lean on.

Step 1 — one shared tile source with a byte cache and single-flight. New crates/viberfox/src/systems/tile_source.rs:

  • async fn tile_bytes(key: &TileKey, template: &str) -> Result<Arc<Vec<u8>>, String> — the single entry point. Order: in-memory cache → in-flight coalesce → platform::storageplatform::http. Cache and in-flight table live in a static OnceLock<Mutex<…>> (not a Bevy Resource) because the callers are detached tasks that hold no World access.
  • The cache is bounded in bytes, LRU, with the ceiling a named const (start at 32 MiB — comfortably the live z14 neighbourhood of every consumer at ~500 KB/tile, and a number a later measurement can move). Byte-bounded, not entry-bounded, so a dense tile cannot blow the budget.
  • Single-flight: a HashMap<CacheKey, Vec<oneshot-ish sender>>; the first caller fetches, the rest await the same result. CacheKey must carry the TileSourceKind for the same reason tile_cache_key does (tile_loader.rs:158) — a source switch must not serve MVT bytes to the PNG decoder.
  • load_tile_image (tile_loader.rs:169) becomes a thin wrapper over it, so fetch_tile_payload, load_globe_tile_rgba, map_geometry, navigation::solve and routing all inherit the cache with no call-site change.

Step 2 — parse each tile body once per task. Add pub struct TileData { reader: Reader, layers: Vec<Layer> } to vector_tiles.rs with a TileData::parse(&[u8]) constructor, and change the six &[u8]-taking functions to take &TileData: render_mvt (:249), tessellate_surfaces (:724), tessellate_roads (:842), ocean_mask (:1198), water_shore_field (:1277), vegetation::scatter_vegetation (:138), and RoadGraph::add_tile (routing.rs:424). Keep &[u8] shims for the existing fixture tests. Then:

  • map_geometry::spawn_geometry_task parses once and passes &TileData to all four (map_geometry.rs:136-172);
  • load_globe_tile_rgba parses once for render_mvt + ocean_mask (tile_loader.rs:302-305);
  • inside render_mvt, memoise get_features per layer index for the duration of the call, so streets and buildings decode once each instead of twice (vector_tiles.rs:274-329).

Step 3 — finish the ADR-033 worker split. Move the CPU half of each loader in the table above onto platform::workers::submit, following the map_stream::spawn_map_tile shape (map_stream.rs:296-343): await the fetch on IoTaskPool, submit the pure-CPU remainder, push to the results sink from the worker. That means splitting load_globe_tile_rgba into a fetch half and a TilePayload-style decode half (the pattern TilePayload already establishes, tile_loader.rs:203-230), the same for spawn_tile_task, hoisting parse_pois + the marker mesh build out of build_layer_cell, and hoisting parse_vehicle_positions/parse_line_map and RoadGraph::add_tile/finish out of their futures. In navigation::solve, replace the serial per-tile loop (navigation.rs:483-501) with a bounded concurrent fetch (reuse Dispatcher, or a simple chunked join_all) feeding the worker-side graph build.

Step 4 — one generic streamer. New crates/viberfox/src/platform/stream.rs holding CellStream<K, R> that owns what the table in Problem 4 lists: requested: HashSet<K>, failed: HashMap<K, (u32, Instant)> with backoff, results: Arc<Mutex<Vec<(K, R)>>>, an embedded Dispatcher<K>, despawn_queue: Vec<Entity> with a caller-supplied per-frame cap, and generation: u32 with a sync(gen) -> bool that reports a re-anchor. Adopt it in map_stream, map_geometry, globe, lod22, data_layers and osm_buildings' far-cell half, deleting the hand-rolled fields. This is where map_stream gains its concurrency bound (Problem 3) and where lod22 and data_layers gain their despawn caps (Problem 4) — as a consequence of using the shared type, not as separate patches.

Step 5 — the small ones. Cache the desired-set computation in MapStream and recompute only when (ctx, cty, current_zoom) changes (map_stream.rs:419-450); derive Copy on TileKey (vibe_core/src/world.rs:6) and drop the now-redundant .clone()s in the hot loops.

Acceptance criteria

  • A single tile_source::tile_bytes is the only path to tile bytes; grep -c "storage::get\|http::fetch" over tile_loader.rs shows the fetch happening in exactly one place, and map_geometry, globe, navigation, map_stream and routing all reach bytes through it.
  • A unit test proves single-flight: two concurrent tile_bytes calls for the same key against a stub source issue one underlying fetch and both receive identical bytes.
  • A unit test proves the byte cache is bounded: inserting more than the ceiling evicts least-recently-used and never exceeds the ceiling.
  • A unit test over update_map_stream's desired-set arithmetic asserts the fan-out numbers: the z10–z17 clipmap at DEFAULT_ANCHOR_ZOOM yields 424 requests, and after SubTile::resolve those collapse to a strictly smaller count of distinct source tiles (assert the exact number the arithmetic gives, so a future radius/zoom change has to restate it).
  • vector_tiles::TileData exists; tessellate_surfaces, tessellate_roads, scatter_vegetation, water_shore_field, render_mvt, ocean_mask and RoadGraph::add_tile all take it, and grep -c "Reader::new" crates/viberfox/src/systems/ outside TileData::parse and #[cfg(test)] is 0.
  • map_geometry::spawn_geometry_task and load_globe_tile_rgba each parse their body exactly once — asserted by a test that parses the Groningen fixture once and checks the outputs match the per-call versions byte-for-byte (renders_a_non_blank_tile, shore_field_measures_distance_into_the_water and the road/surface tessellation tests must all still pass unchanged).
  • Every loader that does CPU work does it under platform::workers::submit. Verifiable by inspection of the five sites named in the Problem table; the IoTaskPool future in each contains only awaits and the submit call.
  • navigation::solve fetches corridor tiles concurrently under an explicit bound, and its graph build runs on platform::workers.
  • map_stream::update_map_stream dispatches through a Dispatcher/CellStream bound, and the bound is a named const with a comment saying why that number.
  • platform::stream::CellStream exists and is used by map_stream, map_geometry, globe, lod22, data_layers and osm_buildings; each of those resources loses its own requested/failed/results/despawn_queue/generation fields.
  • lod22's and data_layers' re-anchor teardown goes through the shared despawn queue under a per-frame cap, like the other four.
  • MapStream's desired set is recomputed only when the camera's tile centre or the LOD zoom changes.
  • TileKey is Copy.
  • Behaviour is unchanged: cargo test -p viberfox --bin viberfox passes, including the existing fixture tests, integer_water_ramp_matches_the_float_one, classifies_raster_and_vector_templates, cache_keys_separate_by_kind and the routing tests.
  • Every const this branch adds or changes carries a comment giving the reason for the number, per the conventions in the surrounding code.

Verification

Runnable here and in CI:

cargo check -p vibe_core
cargo check -p viberfox
cargo test -p viberfox --bin viberfox          # --lib fails; there is no lib target
cargo check --workspace --all-targets          # what CI's check lane runs

Note cargo check does not compile cfg(test) code — run the tests, not just the check.

Grep-level checks for the DRY criteria:

grep -rn "Reader::new" crates/viberfox/src/systems/          # only TileData::parse + tests
grep -rn "IoTaskPool::get" crates/viberfox/src/systems/      # each future: awaits + submit only
grep -rn "Arc<Mutex<Vec<\|requested: HashSet\|despawn_queue" crates/viberfox/src/systems/

The route test prints a full turn list and must still solve after the TileData/concurrent-fetch changes:

cargo test -p viberfox --bin viberfox solves_a_real_route -- --ignored --nocapture

Workstation only — this container has no GPU and no Vulkan driver, so --shot and cargo shots cannot run here. These are the checks that actually confirm "faster", and someone with a workstation must run them:

  • ShotMetrics::settled_s is the streaming-latency number this branch is trying to move (shot_harness.rs:252, and it is already a --csv column). Capture it before and after on the same preset: cargo run -p viberfox --profile dev-bevy -- --shot /tmp/a.png --at 53.2194,6.5665,1200 --look=-45,0 --csv /tmp/before.csv, then the same on the branch. A fresh-anchor capture is exactly the 424-request burst.
  • cargo shots on the full manifest, to confirm the six README views are pixel-unchanged — step 2 rewrites the tessellation and rasterisation call paths, and a wrong TileData hand-off shows up as missing water, missing roads or missing trees.
  • The web build's real cost: tools/cdp_probe.ts --gpu --profile against the real adapter (not SwiftShader — docs/notes/wasm-performance.md says why that distinction is load-bearing), to confirm the main-thread profile is still free of serde_json and now also free of mvt_reader/tiny-skia/image frames.
  • A pan and a re-anchor on a weak iGPU, watching for VK_ERROR_DEVICE_LOST: step 4 changes when lod22 and data_layers free their entities.

If the numbers land, update docs/notes/tile-raster-cost.md — its "The open question" section asks whether to move rasterisation off the CPU, and a measured drop in settled_s from deduplication is evidence bearing on that question.

Out of scope

  • Moving rasterisation to the GPU. That is the open question docs/notes/tile-raster-cost.md records as deferred, and it must survive wasm32 plus the pure-Rust constraint. This branch reduces how often the CPU rasteriser runs; it does not replace it.
  • Typed structs in place of serde_json::Value for the 3DBAG and Overpass parses (lod22.rs:459, osm_buildings.rs:108 and :1845, data_layers.rs:1336, transit_vehicles.rs:566). docs/notes/wasm-performance.md names this as the next win for 3DBAG streaming latency, but it is a different data source from the tile pipeline and a large diff of its own. Step 3 moves that work off the main thread; it does not make it cheaper. Worth its own issue.
  • An IndexedDB implementation for platform::storage on wasm (platform/storage.rs:77-91). Its absence is what turns Problem 1 into repeated network fetches on web, and the in-memory cache in step 1 substantially mitigates it within a session — but the real fix is ADR-033's own pending work, not this branch.
  • Retiring the legacy region ground plane. load_region_tiles/TileCache/RegionTile (tile_loader.rs:345, registered at main.rs:637) run a second, complete copy of the tile pipeline for the single region vibe_sim seeds (crates/vibe_sim/src/db.rs:80-99) — a ground quad that map_stream already covers, and which CLAUDE.md says the streaming tier "supersedes". This branch routes it through the shared cache and the worker pool along with everyone else (steps 1 and 3), but deleting the region mesh is a visible change to the scene that cannot be checked in this container, and it is an ADR-020/029 cleanup rather than a loader change. Pull it in if you want it.
  • Elevation / terrain (docs/adr/038) and any change to what the loaders load. Byte-for-byte identical output is a criterion, not a nice-to-have.
  • New tuning of the existing per-frame build/despawn/upload caps. They are device-loss guards, documented as such; this branch makes lod22 and data_layers obey the pattern, and changes no number that already exists.

Open questions

None — the scope above is decided. Two judgement calls are worth flagging so they can be overridden rather than discovered:

  • The byte-cache ceiling starts at a named 32 MiB const rather than a measured figure. Nobody has measured the live tile working set, and measuring is workstation work that belongs after this label moves; the const is placed so one edit moves it.
  • Only tile bytes are cached across tasks, not parsed Readers. A parsed Reader holds geo_types f32 geometry for every feature, and nothing in the tree measures how much that is — so caching bytes (whose size is exactly known) is the safe half, and the parse duplication is removed within a task by TileData instead. A cross-task Arc<TileData> cache would additionally collapse the ~179 repeat parses per anchor and is the obvious follow-up, once someone has a number for the memory.

Branch: perf/11-tile-pipeline-dedup

Original request

Find performance issues in the generic data loading mechanism and make it faster and DRY. Would be nice to be able to make the data loaders more generic if possible.

🤖 Refined by the viberfox issue agent. Reply with @agent refine and what is wrong to have this rewritten.

## Problem The client has **nine independent data loaders** (map tile textures, vector surface geometry, the globe quadtree, 3DBAG LoD2.2, OSM/Overpass buildings, POI data layers, routing, transit vehicles, weather). They were migrated onto the ADR-033 seams (`platform::http`, `platform::storage`, `platform::work::Dispatcher`, `platform::workers`) one at a time, and the migration is uneven. Five concrete problems fall out of that, all citable: ### 1. The same source tile is fetched, read and re-parsed many times over There is **no in-memory cache and no single-flight** anywhere in the tile path. Every consumer calls `tile_loader::load_tile_image` (`crates/viberfox/src/systems/tile_loader.rs:169`), which checks the on-disk blob cache and otherwise hits the network. Two consumers asking for the same tile in the same frame both miss and both fetch, because each streamer de-dups only against *its own* `requested` set. The overzoom fan-out makes this large. A fresh anchor opens at `DEFAULT_ANCHOR_ZOOM = 17` (`map_stream.rs:248`) and `update_map_stream` builds a clipmap of `MIN_ZOOM..=current_zoom` = z10–z17 (`map_stream.rs:59`, `:424`), radius `LOAD_RADIUS = 4` at the finest zoom and `COARSE_RADIUS = 3` below (`:53`, `:56`, `:429`) — **9×9 + 7×(7×7) = 424 tile requests**. Shortbread tops out at `MAX_SOURCE_ZOOM = 14` (`vector_tiles.rs:40`), so the 179 requests at z15/z16/z17 are all resolved by `SubTile::resolve` to a z14 ancestor (`vector_tiles.rs:67`, reached via `tile_loader.rs:240`). Those 179 requests resolve to only a few dozen distinct z14 bodies — and the z14 ring (49 tiles, radius 3 around the same camera) requests essentially all of them in the same frame anyway. Each of those 179 requests independently: - reads the ~500 KB body (a `std::fs::read` per request natively — `platform/storage.rs:56`); - **re-fetches it over the network on the web build**, because the wasm `storage` impl is still a `None`-returning stub (`platform/storage.rs:83-90`); - re-runs `Reader::new(bytes.to_vec())` — a full copy plus a prost decode of the whole tile (`vector_tiles.rs:250`). Then the same z14 bodies are fetched *again* by the other consumers: - `map_geometry` streams 3×3 cells at the same `GEOMETRY_ZOOM = MAX_SOURCE_ZOOM` (`map_geometry.rs:51`, `:55`) and parses each body **four more times** — `tessellate_surfaces` (`:136`), `tessellate_roads` (`:142`), `scatter_vegetation` (`:153`) and `water_shore_field` (`:166`), each constructing its own `Reader` (`vector_tiles.rs:725`, `:843`, `vegetation.rs:142`, `vector_tiles.rs:1278`); - routing fetches up to `MAX_ROUTE_TILES = 96` z14 tiles per solve (`routing.rs:69`, `navigation.rs:485`); - `load_globe_tile_rgba` parses each globe tile **twice** — once in `render_mvt`, once in `ocean_mask` (`tile_loader.rs:302`, `:305`). Per `docs/notes/tile-raster-cost.md` (measured 2026-07-25), MVT parse is ~2.7 ms and `get_features` ~4 ms on the Groningen z14 fixture, against ~121 ms to rasterise. So the duplicated *parse* is not the headline cost — but the duplicated *fetch* is unbounded bandwidth on web, and the duplicated disk read is 179 × 500 KB per anchor natively. Smaller, in the same family: `render_mvt` walks `PAINT_ORDER`, which lists `streets` twice (`vector_tiles.rs:181-182`, casing then fill) and `buildings` twice (`:190`, `:191`), and calls `reader.get_features(idx)` inside that loop (`:278`). `mvt_reader::Reader::get_features` decodes geometry and properties into a fresh `Vec<Feature>` on every call (`mvt-reader-2.4.0/src/lib.rs:208`), so the two heaviest layers of every tile are decoded twice per render. ### 2. Half the loaders still do their CPU work on the wasm main thread `docs/notes/wasm-performance.md` records the fix: on wasm Bevy disables `multi_threaded`, so `IoTaskPool` *is* the main thread, and parse/mesh work must be handed to `platform::workers` (`platform/workers.rs:9-25`). That was done for `map_stream` (`:308`), `map_geometry` (`:133`), `lod22` (`:228`) and `osm_buildings` (`:543`, `:640`). It was **not** done for: | Loader | CPU work still inside the `IoTaskPool` future | |---|---| | `globe.rs:870-888` | `load_globe_tile_rgba` (MVT rasterise + `ocean_mask`, or a PNG decode + `paint_water_alpha_by_colour`) **and** `decode_globe_tile`'s mip chain — up to `QT_FETCH_BUDGET = 24` in flight (`globe.rs:104`) | | `tile_loader.rs:60-83` | `load_tile_rgba` + `build_mip_chain` | | `data_layers.rs:530-537` | `build_layer_cell` → `parse_pois` (`serde_json::Value`, `:1336`) + the merged marker mesh (`:1273-1308`) | | `transit_vehicles.rs:318` | `parse_vehicle_positions` (GTFS-RT protobuf) + `parse_line_map` (`serde_json::Value`, `:566`) every `POLL_INTERVAL` | | `navigation.rs:445-452` | the whole `solve`: `RoadGraph::add_tile` per tile (its own `Reader::new`, `routing.rs:424`) plus `graph.finish()`'s segment splitting | `navigation::solve` also fetches its corridor tiles **serially** — a `for` loop with an `.await` per tile (`navigation.rs:483-501`) — so a 96-tile route pays 96 sequential round trips even natively. ### 3. `map_stream` is the one streamer with no concurrency bound Every other streamer gates admission: `tile_loader` `Dispatcher::new(TILE_WORKERS = 3, …)` (`tile_loader.rs:15`, `:46`), `lod22` `LOD22_CONCURRENCY = 3` (`lod22.rs:44`, `:156`), `data_layers` serial + Overpass-throttled (`data_layers.rs:464`), `osm_buildings` serial (`osm_buildings.rs:464`), `map_geometry` `SURFACE_MAX_IN_FLIGHT = 8` (`map_geometry.rs:82`), `globe` `QT_FETCH_BUDGET = 24` (`globe.rs:1238`). `map_stream` spawns one detached task per newly-wanted tile with **no cap at all** (`map_stream.rs:486-495`), so a fresh anchor or a re-anchor releases all 424 at once. `platform::work::Dispatcher` exists for exactly this (`platform/work.rs:18`) and `map_stream` is the only tile streamer that does not use it. ### 4. Six streamers hand-roll the same skeleton The same five-or-six fields, with the same semantics and slightly different bugs, appear in: | | requested set | failed + backoff | results sink | in-flight bound | despawn queue + per-frame cap | anchor generation | |---|---|---|---|---|---|---| | `map_stream.rs:130-145` | ✓ | ✓ `(u32, Instant)` | `Arc<Mutex<HashMap>>` | — | ✓ 12/frame | ✓ | | `map_geometry.rs:96-112` | ✓ | — | `Arc<Mutex<Vec>>` | ✓ counter | ✓ 4/frame | ✓ | | `globe.rs:165-184` | `pending` | ✓ `(u32, Instant)` | `Arc<Mutex<Vec>>` | ✓ budget | ✓ 8/frame | ✓ | | `lod22.rs:99-114` | ✓ | ✓ `u32` | `Arc<Mutex<Vec>>` | ✓ `Dispatcher` | **✗ — all at once** (`:138-140`) | ✓ | | `data_layers.rs:266-282`, `:328-329` | ✓ | ✓ `u32` + `retry_at` | `Arc<Mutex<Vec>>` | ✓ `Dispatcher` | **✗ — all at once** (`:441-451`) | ✓ | | `osm_buildings.rs:250-278` | `far_requested` | ✓ `u32` | `Arc<Mutex<Vec>>` | ✓ `Dispatcher` | — | ✓ | The two ✗ cells are the interesting ones: CLAUDE.md's known-issues section names mass despawn in one command buffer as the `VK_ERROR_DEVICE_LOST` pattern, and four streamers have a per-frame despawn cap for that reason. `lod22`'s re-anchor path despawns every cell in one frame (`lod22.rs:138-140`, up to `(2·LOD22_RADIUS+1)² = 25` merged 3DBAG meshes), and `data_layers`' re-anchor despawns every marker entity of every enabled layer in one frame (`data_layers.rs:441-451`). ### 5. Per-frame waste in `update_map_stream` The 424-entry `wanted: HashMap<TileKey, f32>` and the 424-entry `spawn_order: Vec<TileKey>` are rebuilt from scratch **every frame**, unconditionally (`map_stream.rs:419-450`), as is the `stale` scan over `loaded` (`:531-536`). The desired set only changes when the camera crosses a tile boundary or the LOD zoom steps. `TileKey` is 20 bytes of plain data but is `Clone`-not-`Copy` (`crates/vibe_core/src/world.rs:6`), so each rebuild is also ~850 `TileKey` clones. ## Approach Five steps, each independently compilable and testable. Order matters: step 1 is what the rest lean on. **Step 1 — one shared tile source with a byte cache and single-flight.** New `crates/viberfox/src/systems/tile_source.rs`: - `async fn tile_bytes(key: &TileKey, template: &str) -> Result<Arc<Vec<u8>>, String>` — the single entry point. Order: in-memory cache → in-flight coalesce → `platform::storage` → `platform::http`. Cache and in-flight table live in a `static OnceLock<Mutex<…>>` (not a Bevy `Resource`) because the callers are detached tasks that hold no `World` access. - The cache is bounded **in bytes**, LRU, with the ceiling a named `const` (start at 32 MiB — comfortably the live z14 neighbourhood of every consumer at ~500 KB/tile, and a number a later measurement can move). Byte-bounded, not entry-bounded, so a dense tile cannot blow the budget. - Single-flight: a `HashMap<CacheKey, Vec<oneshot-ish sender>>`; the first caller fetches, the rest await the same result. `CacheKey` must carry the `TileSourceKind` for the same reason `tile_cache_key` does (`tile_loader.rs:158`) — a source switch must not serve MVT bytes to the PNG decoder. - `load_tile_image` (`tile_loader.rs:169`) becomes a thin wrapper over it, so `fetch_tile_payload`, `load_globe_tile_rgba`, `map_geometry`, `navigation::solve` and `routing` all inherit the cache with no call-site change. **Step 2 — parse each tile body once per task.** Add `pub struct TileData { reader: Reader, layers: Vec<Layer> }` to `vector_tiles.rs` with a `TileData::parse(&[u8])` constructor, and change the six `&[u8]`-taking functions to take `&TileData`: `render_mvt` (`:249`), `tessellate_surfaces` (`:724`), `tessellate_roads` (`:842`), `ocean_mask` (`:1198`), `water_shore_field` (`:1277`), `vegetation::scatter_vegetation` (`:138`), and `RoadGraph::add_tile` (`routing.rs:424`). Keep `&[u8]` shims for the existing fixture tests. Then: - `map_geometry::spawn_geometry_task` parses once and passes `&TileData` to all four (`map_geometry.rs:136-172`); - `load_globe_tile_rgba` parses once for `render_mvt` + `ocean_mask` (`tile_loader.rs:302-305`); - inside `render_mvt`, memoise `get_features` per layer index for the duration of the call, so `streets` and `buildings` decode once each instead of twice (`vector_tiles.rs:274-329`). **Step 3 — finish the ADR-033 worker split.** Move the CPU half of each loader in the table above onto `platform::workers::submit`, following the `map_stream::spawn_map_tile` shape (`map_stream.rs:296-343`): await the fetch on `IoTaskPool`, `submit` the pure-CPU remainder, push to the results sink from the worker. That means splitting `load_globe_tile_rgba` into a fetch half and a `TilePayload`-style decode half (the pattern `TilePayload` already establishes, `tile_loader.rs:203-230`), the same for `spawn_tile_task`, hoisting `parse_pois` + the marker mesh build out of `build_layer_cell`, and hoisting `parse_vehicle_positions`/`parse_line_map` and `RoadGraph::add_tile`/`finish` out of their futures. In `navigation::solve`, replace the serial per-tile loop (`navigation.rs:483-501`) with a bounded concurrent fetch (reuse `Dispatcher`, or a simple chunked `join_all`) feeding the worker-side graph build. **Step 4 — one generic streamer.** New `crates/viberfox/src/platform/stream.rs` holding `CellStream<K, R>` that owns what the table in Problem 4 lists: `requested: HashSet<K>`, `failed: HashMap<K, (u32, Instant)>` with backoff, `results: Arc<Mutex<Vec<(K, R)>>>`, an embedded `Dispatcher<K>`, `despawn_queue: Vec<Entity>` with a caller-supplied per-frame cap, and `generation: u32` with a `sync(gen) -> bool` that reports a re-anchor. Adopt it in `map_stream`, `map_geometry`, `globe`, `lod22`, `data_layers` and `osm_buildings`' far-cell half, deleting the hand-rolled fields. This is where `map_stream` gains its concurrency bound (Problem 3) and where `lod22` and `data_layers` gain their despawn caps (Problem 4) — as a consequence of using the shared type, not as separate patches. **Step 5 — the small ones.** Cache the desired-set computation in `MapStream` and recompute only when `(ctx, cty, current_zoom)` changes (`map_stream.rs:419-450`); derive `Copy` on `TileKey` (`vibe_core/src/world.rs:6`) and drop the now-redundant `.clone()`s in the hot loops. ## Acceptance criteria - [ ] A single `tile_source::tile_bytes` is the only path to tile bytes; `grep -c "storage::get\|http::fetch" ` over `tile_loader.rs` shows the fetch happening in exactly one place, and `map_geometry`, `globe`, `navigation`, `map_stream` and `routing` all reach bytes through it. - [ ] A unit test proves single-flight: two concurrent `tile_bytes` calls for the same key against a stub source issue **one** underlying fetch and both receive identical bytes. - [ ] A unit test proves the byte cache is bounded: inserting more than the ceiling evicts least-recently-used and never exceeds the ceiling. - [ ] A unit test over `update_map_stream`'s desired-set arithmetic asserts the fan-out numbers: the z10–z17 clipmap at `DEFAULT_ANCHOR_ZOOM` yields 424 requests, and after `SubTile::resolve` those collapse to a strictly smaller count of distinct source tiles (assert the exact number the arithmetic gives, so a future radius/zoom change has to restate it). - [ ] `vector_tiles::TileData` exists; `tessellate_surfaces`, `tessellate_roads`, `scatter_vegetation`, `water_shore_field`, `render_mvt`, `ocean_mask` and `RoadGraph::add_tile` all take it, and `grep -c "Reader::new" crates/viberfox/src/systems/` outside `TileData::parse` and `#[cfg(test)]` is 0. - [ ] `map_geometry::spawn_geometry_task` and `load_globe_tile_rgba` each parse their body exactly once — asserted by a test that parses the Groningen fixture once and checks the outputs match the per-call versions byte-for-byte (`renders_a_non_blank_tile`, `shore_field_measures_distance_into_the_water` and the road/surface tessellation tests must all still pass unchanged). - [ ] Every loader that does CPU work does it under `platform::workers::submit`. Verifiable by inspection of the five sites named in the Problem table; the `IoTaskPool` future in each contains only `await`s and the `submit` call. - [ ] `navigation::solve` fetches corridor tiles concurrently under an explicit bound, and its graph build runs on `platform::workers`. - [ ] `map_stream::update_map_stream` dispatches through a `Dispatcher`/`CellStream` bound, and the bound is a named `const` with a comment saying why that number. - [ ] `platform::stream::CellStream` exists and is used by `map_stream`, `map_geometry`, `globe`, `lod22`, `data_layers` and `osm_buildings`; each of those resources loses its own `requested`/`failed`/`results`/`despawn_queue`/`generation` fields. - [ ] `lod22`'s and `data_layers`' re-anchor teardown goes through the shared despawn queue under a per-frame cap, like the other four. - [ ] `MapStream`'s desired set is recomputed only when the camera's tile centre or the LOD zoom changes. - [ ] `TileKey` is `Copy`. - [ ] Behaviour is unchanged: `cargo test -p viberfox --bin viberfox` passes, including the existing fixture tests, `integer_water_ramp_matches_the_float_one`, `classifies_raster_and_vector_templates`, `cache_keys_separate_by_kind` and the routing tests. - [ ] Every `const` this branch adds or changes carries a comment giving the reason for the number, per the conventions in the surrounding code. ## Verification Runnable here and in CI: ```bash cargo check -p vibe_core cargo check -p viberfox cargo test -p viberfox --bin viberfox # --lib fails; there is no lib target cargo check --workspace --all-targets # what CI's check lane runs ``` Note `cargo check` does not compile `cfg(test)` code — run the tests, not just the check. Grep-level checks for the DRY criteria: ```bash grep -rn "Reader::new" crates/viberfox/src/systems/ # only TileData::parse + tests grep -rn "IoTaskPool::get" crates/viberfox/src/systems/ # each future: awaits + submit only grep -rn "Arc<Mutex<Vec<\|requested: HashSet\|despawn_queue" crates/viberfox/src/systems/ ``` The route test prints a full turn list and must still solve after the `TileData`/concurrent-fetch changes: ```bash cargo test -p viberfox --bin viberfox solves_a_real_route -- --ignored --nocapture ``` **Workstation only — this container has no GPU and no Vulkan driver, so `--shot` and `cargo shots` cannot run here.** These are the checks that actually confirm "faster", and someone with a workstation must run them: - `ShotMetrics::settled_s` is the streaming-latency number this branch is trying to move (`shot_harness.rs:252`, and it is already a `--csv` column). Capture it before and after on the same preset: `cargo run -p viberfox --profile dev-bevy -- --shot /tmp/a.png --at 53.2194,6.5665,1200 --look=-45,0 --csv /tmp/before.csv`, then the same on the branch. A fresh-anchor capture is exactly the 424-request burst. - `cargo shots` on the full manifest, to confirm the six README views are pixel-unchanged — step 2 rewrites the tessellation and rasterisation call paths, and a wrong `TileData` hand-off shows up as missing water, missing roads or missing trees. - The web build's real cost: `tools/cdp_probe.ts --gpu --profile` against the real adapter (not SwiftShader — `docs/notes/wasm-performance.md` says why that distinction is load-bearing), to confirm the main-thread profile is still free of `serde_json` and now also free of `mvt_reader`/`tiny-skia`/`image` frames. - A pan and a re-anchor on a weak iGPU, watching for `VK_ERROR_DEVICE_LOST`: step 4 changes when `lod22` and `data_layers` free their entities. If the numbers land, update `docs/notes/tile-raster-cost.md` — its "The open question" section asks whether to move rasterisation off the CPU, and a measured drop in `settled_s` from deduplication is evidence bearing on that question. ## Out of scope - **Moving rasterisation to the GPU.** That is the open question `docs/notes/tile-raster-cost.md` records as deferred, and it must survive wasm32 plus the pure-Rust constraint. This branch reduces how *often* the CPU rasteriser runs; it does not replace it. - **Typed structs in place of `serde_json::Value`** for the 3DBAG and Overpass parses (`lod22.rs:459`, `osm_buildings.rs:108` and `:1845`, `data_layers.rs:1336`, `transit_vehicles.rs:566`). `docs/notes/wasm-performance.md` names this as the next win for 3DBAG streaming latency, but it is a different data source from the tile pipeline and a large diff of its own. Step 3 moves that work off the main thread; it does not make it cheaper. Worth its own issue. - **An IndexedDB implementation for `platform::storage` on wasm** (`platform/storage.rs:77-91`). Its absence is what turns Problem 1 into repeated *network* fetches on web, and the in-memory cache in step 1 substantially mitigates it within a session — but the real fix is ADR-033's own pending work, not this branch. - **Retiring the legacy region ground plane.** `load_region_tiles`/`TileCache`/`RegionTile` (`tile_loader.rs:345`, registered at `main.rs:637`) run a second, complete copy of the tile pipeline for the single region `vibe_sim` seeds (`crates/vibe_sim/src/db.rs:80-99`) — a ground quad that `map_stream` already covers, and which CLAUDE.md says the streaming tier "supersedes". This branch routes it through the shared cache and the worker pool along with everyone else (steps 1 and 3), but **deleting** the region mesh is a visible change to the scene that cannot be checked in this container, and it is an ADR-020/029 cleanup rather than a loader change. Pull it in if you want it. - **Elevation / terrain** (`docs/adr/038`) and any change to what the loaders load. Byte-for-byte identical output is a criterion, not a nice-to-have. - **New tuning of the existing per-frame build/despawn/upload caps.** They are device-loss guards, documented as such; this branch makes `lod22` and `data_layers` obey the pattern, and changes no number that already exists. ## Open questions None — the scope above is decided. Two judgement calls are worth flagging so they can be overridden rather than discovered: - The byte-cache ceiling starts at a named 32 MiB const rather than a measured figure. Nobody has measured the live tile working set, and measuring is workstation work that belongs after this label moves; the const is placed so one edit moves it. - Only tile *bytes* are cached across tasks, not parsed `Reader`s. A parsed `Reader` holds `geo_types` f32 geometry for every feature, and nothing in the tree measures how much that is — so caching bytes (whose size is exactly known) is the safe half, and the parse duplication is removed *within* a task by `TileData` instead. A cross-task `Arc<TileData>` cache would additionally collapse the ~179 repeat parses per anchor and is the obvious follow-up, once someone has a number for the memory. --- Branch: `perf/11-tile-pipeline-dedup` <details><summary>Original request</summary> Find performance issues in the generic data loading mechanism and make it faster and DRY. Would be nice to be able to make the data loaders more generic if possible. </details> <sub>🤖 Refined by the viberfox issue agent. Reply with **@agent refine** and what is wrong to have this rewritten.</sub>
jeroen added this to the Viberfox project 2026-07-30 12:31:36 +00:00
Collaborator

🤖 Refining this before anything gets built — reading the code, no branch, no commits. If it turns out I'd have to guess at the requirements I'll stop and ask instead.

🤖 Refining this before anything gets built — reading the code, no branch, no commits. If it turns out I'd have to guess at the requirements I'll stop and ask instead.
Collaborator

🤖 My refinement session was interrupted — the watcher restarted. Requeued; I'll start over on this issue shortly.

🤖 My refinement session was interrupted — the watcher restarted. Requeued; I'll start over on this issue shortly.
Collaborator

🤖 Refining this before anything gets built — reading the code, no branch, no commits. If it turns out I'd have to guess at the requirements I'll stop and ask instead.

🤖 Refining this before anything gets built — reading the code, no branch, no commits. If it turns out I'd have to guess at the requirements I'll stop and ask instead.
Collaborator

🤖 Refined. The description above is now the spec; I'll pick it up for implementation on the next poll.

If the direction is wrong, say so now — @agent refine with a correction rewrites it, and agent:skip stops me.


🤖 viberfox issue agent on the VPS — 61 turns · $4.98 · session 543f520e-8e01-497a-9559-1674055c9e68. Reply with @agent to continue this session.

🤖 **Refined.** The description above is now the spec; I'll pick it up for implementation on the next poll. If the direction is wrong, say so now — **@agent refine** with a correction rewrites it, and `agent:skip` stops me. --- <sub>🤖 viberfox issue agent on the VPS — 61 turns · $4.98 · session `543f520e-8e01-497a-9559-1674055c9e68`. Reply with **@agent** to continue this session.</sub>
Collaborator

🤖 Picked this up — working on branch perf/11-tile-pipeline-dedup in a fresh worktree on the VPS. I'll report back here when I'm done.

Label an issue agent:skip to keep me off it, or driven to keep it in a session you are driving.

🤖 Picked this up — working on branch `perf/11-tile-pipeline-dedup` in a fresh worktree on the VPS. I'll report back here when I'm done. <sub>Label an issue `agent:skip` to keep me off it, or `driven` to keep it in a session you are driving.</sub>
Collaborator

The session was killed after hitting the 45 minute limit.

Branch perf/11-tile-pipeline-dedup (pushed)

2c8f79a refactor(loaders): one shared cell streamer for all six tile streamers
35f7a49 perf(tiles): dedup the tile pipeline behind one cached byte source

🤖 viberfox issue agent on the VPS — . Reply with @agent to continue this session.

The session was killed after hitting the 45 minute limit. **Branch** `perf/11-tile-pipeline-dedup` (pushed) ``` 2c8f79a refactor(loaders): one shared cell streamer for all six tile streamers 35f7a49 perf(tiles): dedup the tile pipeline behind one cached byte source ``` --- <sub>🤖 viberfox issue agent on the VPS — . Reply with **@agent** to continue this session.</sub>
Sign in to join this conversation.
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
jeroen/cartopolis#11
No description provided.