Performance improvements refactor #11
Labels
No labels
agent
agent:ci
agent:done
agent:failed
agent:needs-input
agent:refined
agent:refining
agent:running
agent:skip
autonomous
driven
local
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
jeroen/cartopolis#11
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
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 ownrequestedset.The overzoom fan-out makes this large. A fresh anchor opens at
DEFAULT_ANCHOR_ZOOM = 17(map_stream.rs:248) andupdate_map_streambuilds a clipmap ofMIN_ZOOM..=current_zoom= z10–z17 (map_stream.rs:59,:424), radiusLOAD_RADIUS = 4at the finest zoom andCOARSE_RADIUS = 3below (:53,:56,:429) — 9×9 + 7×(7×7) = 424 tile requests. Shortbread tops out atMAX_SOURCE_ZOOM = 14(vector_tiles.rs:40), so the 179 requests at z15/z16/z17 are all resolved bySubTile::resolveto a z14 ancestor (vector_tiles.rs:67, reached viatile_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:
std::fs::readper request natively —platform/storage.rs:56);storageimpl is still aNone-returning stub (platform/storage.rs:83-90);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_geometrystreams 3×3 cells at the sameGEOMETRY_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) andwater_shore_field(:166), each constructing its ownReader(vector_tiles.rs:725,:843,vegetation.rs:142,vector_tiles.rs:1278);MAX_ROUTE_TILES = 96z14 tiles per solve (routing.rs:69,navigation.rs:485);load_globe_tile_rgbaparses each globe tile twice — once inrender_mvt, once inocean_mask(tile_loader.rs:302,:305).Per
docs/notes/tile-raster-cost.md(measured 2026-07-25), MVT parse is ~2.7 ms andget_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_mvtwalksPAINT_ORDER, which listsstreetstwice (vector_tiles.rs:181-182, casing then fill) andbuildingstwice (:190,:191), and callsreader.get_features(idx)inside that loop (:278).mvt_reader::Reader::get_featuresdecodes geometry and properties into a freshVec<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.mdrecords the fix: on wasm Bevy disablesmulti_threaded, soIoTaskPoolis the main thread, and parse/mesh work must be handed toplatform::workers(platform/workers.rs:9-25). That was done formap_stream(:308),map_geometry(:133),lod22(:228) andosm_buildings(:543,:640). It was not done for:IoTaskPoolfutureglobe.rs:870-888load_globe_tile_rgba(MVT rasterise +ocean_mask, or a PNG decode +paint_water_alpha_by_colour) anddecode_globe_tile's mip chain — up toQT_FETCH_BUDGET = 24in flight (globe.rs:104)tile_loader.rs:60-83load_tile_rgba+build_mip_chaindata_layers.rs:530-537build_layer_cell→parse_pois(serde_json::Value,:1336) + the merged marker mesh (:1273-1308)transit_vehicles.rs:318parse_vehicle_positions(GTFS-RT protobuf) +parse_line_map(serde_json::Value,:566) everyPOLL_INTERVALnavigation.rs:445-452solve:RoadGraph::add_tileper tile (its ownReader::new,routing.rs:424) plusgraph.finish()'s segment splittingnavigation::solvealso fetches its corridor tiles serially — aforloop with an.awaitper tile (navigation.rs:483-501) — so a 96-tile route pays 96 sequential round trips even natively.3.
map_streamis the one streamer with no concurrency boundEvery other streamer gates admission:
tile_loaderDispatcher::new(TILE_WORKERS = 3, …)(tile_loader.rs:15,:46),lod22LOD22_CONCURRENCY = 3(lod22.rs:44,:156),data_layersserial + Overpass-throttled (data_layers.rs:464),osm_buildingsserial (osm_buildings.rs:464),map_geometrySURFACE_MAX_IN_FLIGHT = 8(map_geometry.rs:82),globeQT_FETCH_BUDGET = 24(globe.rs:1238).map_streamspawns 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::Dispatcherexists for exactly this (platform/work.rs:18) andmap_streamis 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:
map_stream.rs:130-145(u32, Instant)Arc<Mutex<HashMap>>map_geometry.rs:96-112Arc<Mutex<Vec>>globe.rs:165-184pending(u32, Instant)Arc<Mutex<Vec>>lod22.rs:99-114u32Arc<Mutex<Vec>>Dispatcher:138-140)data_layers.rs:266-282,:328-329u32+retry_atArc<Mutex<Vec>>Dispatcher:441-451)osm_buildings.rs:250-278far_requestedu32Arc<Mutex<Vec>>DispatcherThe two ✗ cells are the interesting ones: CLAUDE.md's known-issues section names mass despawn in one command buffer as the
VK_ERROR_DEVICE_LOSTpattern, 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)² = 25merged 3DBAG meshes), anddata_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_streamThe 424-entry
wanted: HashMap<TileKey, f32>and the 424-entryspawn_order: Vec<TileKey>are rebuilt from scratch every frame, unconditionally (map_stream.rs:419-450), as is thestalescan overloaded(:531-536). The desired set only changes when the camera crosses a tile boundary or the LOD zoom steps.TileKeyis 20 bytes of plain data but isClone-not-Copy(crates/vibe_core/src/world.rs:6), so each rebuild is also ~850TileKeyclones.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 astatic OnceLock<Mutex<…>>(not a BevyResource) because the callers are detached tasks that hold noWorldaccess.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.HashMap<CacheKey, Vec<oneshot-ish sender>>; the first caller fetches, the rest await the same result.CacheKeymust carry theTileSourceKindfor the same reasontile_cache_keydoes (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, sofetch_tile_payload,load_globe_tile_rgba,map_geometry,navigation::solveandroutingall 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> }tovector_tiles.rswith aTileData::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), andRoadGraph::add_tile(routing.rs:424). Keep&[u8]shims for the existing fixture tests. Then:map_geometry::spawn_geometry_taskparses once and passes&TileDatato all four (map_geometry.rs:136-172);load_globe_tile_rgbaparses once forrender_mvt+ocean_mask(tile_loader.rs:302-305);render_mvt, memoiseget_featuresper layer index for the duration of the call, sostreetsandbuildingsdecode 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 themap_stream::spawn_map_tileshape (map_stream.rs:296-343): await the fetch onIoTaskPool,submitthe pure-CPU remainder, push to the results sink from the worker. That means splittingload_globe_tile_rgbainto a fetch half and aTilePayload-style decode half (the patternTilePayloadalready establishes,tile_loader.rs:203-230), the same forspawn_tile_task, hoistingparse_pois+ the marker mesh build out ofbuild_layer_cell, and hoistingparse_vehicle_positions/parse_line_mapandRoadGraph::add_tile/finishout of their futures. Innavigation::solve, replace the serial per-tile loop (navigation.rs:483-501) with a bounded concurrent fetch (reuseDispatcher, or a simple chunkedjoin_all) feeding the worker-side graph build.Step 4 — one generic streamer. New
crates/viberfox/src/platform/stream.rsholdingCellStream<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 embeddedDispatcher<K>,despawn_queue: Vec<Entity>with a caller-supplied per-frame cap, andgeneration: u32with async(gen) -> boolthat reports a re-anchor. Adopt it inmap_stream,map_geometry,globe,lod22,data_layersandosm_buildings' far-cell half, deleting the hand-rolled fields. This is wheremap_streamgains its concurrency bound (Problem 3) and wherelod22anddata_layersgain 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
MapStreamand recompute only when(ctx, cty, current_zoom)changes (map_stream.rs:419-450); deriveCopyonTileKey(vibe_core/src/world.rs:6) and drop the now-redundant.clone()s in the hot loops.Acceptance criteria
tile_source::tile_bytesis the only path to tile bytes;grep -c "storage::get\|http::fetch"overtile_loader.rsshows the fetch happening in exactly one place, andmap_geometry,globe,navigation,map_streamandroutingall reach bytes through it.tile_bytescalls for the same key against a stub source issue one underlying fetch and both receive identical bytes.update_map_stream's desired-set arithmetic asserts the fan-out numbers: the z10–z17 clipmap atDEFAULT_ANCHOR_ZOOMyields 424 requests, and afterSubTile::resolvethose 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::TileDataexists;tessellate_surfaces,tessellate_roads,scatter_vegetation,water_shore_field,render_mvt,ocean_maskandRoadGraph::add_tileall take it, andgrep -c "Reader::new" crates/viberfox/src/systems/outsideTileData::parseand#[cfg(test)]is 0.map_geometry::spawn_geometry_taskandload_globe_tile_rgbaeach 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_waterand the road/surface tessellation tests must all still pass unchanged).platform::workers::submit. Verifiable by inspection of the five sites named in the Problem table; theIoTaskPoolfuture in each contains onlyawaits and thesubmitcall.navigation::solvefetches corridor tiles concurrently under an explicit bound, and its graph build runs onplatform::workers.map_stream::update_map_streamdispatches through aDispatcher/CellStreambound, and the bound is a namedconstwith a comment saying why that number.platform::stream::CellStreamexists and is used bymap_stream,map_geometry,globe,lod22,data_layersandosm_buildings; each of those resources loses its ownrequested/failed/results/despawn_queue/generationfields.lod22's anddata_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.TileKeyisCopy.cargo test -p viberfox --bin viberfoxpasses, including the existing fixture tests,integer_water_ramp_matches_the_float_one,classifies_raster_and_vector_templates,cache_keys_separate_by_kindand the routing tests.constthis 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:
Note
cargo checkdoes not compilecfg(test)code — run the tests, not just the check.Grep-level checks for the DRY criteria:
The route test prints a full turn list and must still solve after the
TileData/concurrent-fetch changes:Workstation only — this container has no GPU and no Vulkan driver, so
--shotandcargo shotscannot run here. These are the checks that actually confirm "faster", and someone with a workstation must run them:ShotMetrics::settled_sis the streaming-latency number this branch is trying to move (shot_harness.rs:252, and it is already a--csvcolumn). 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 shotson the full manifest, to confirm the six README views are pixel-unchanged — step 2 rewrites the tessellation and rasterisation call paths, and a wrongTileDatahand-off shows up as missing water, missing roads or missing trees.tools/cdp_probe.ts --gpu --profileagainst the real adapter (not SwiftShader —docs/notes/wasm-performance.mdsays why that distinction is load-bearing), to confirm the main-thread profile is still free ofserde_jsonand now also free ofmvt_reader/tiny-skia/imageframes.VK_ERROR_DEVICE_LOST: step 4 changes whenlod22anddata_layersfree 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 insettled_sfrom deduplication is evidence bearing on that question.Out of scope
docs/notes/tile-raster-cost.mdrecords 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.serde_json::Valuefor the 3DBAG and Overpass parses (lod22.rs:459,osm_buildings.rs:108and:1845,data_layers.rs:1336,transit_vehicles.rs:566).docs/notes/wasm-performance.mdnames 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.platform::storageon 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.load_region_tiles/TileCache/RegionTile(tile_loader.rs:345, registered atmain.rs:637) run a second, complete copy of the tile pipeline for the single regionvibe_simseeds (crates/vibe_sim/src/db.rs:80-99) — a ground quad thatmap_streamalready 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.docs/adr/038) and any change to what the loaders load. Byte-for-byte identical output is a criterion, not a nice-to-have.lod22anddata_layersobey 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:
Readers. A parsedReaderholdsgeo_typesf32 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 byTileDatainstead. A cross-taskArc<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-dedupOriginal 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.
🤖 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.
🤖 My refinement session was interrupted — the watcher restarted. Requeued; I'll start over on this issue shortly.
🤖 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.
🤖 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:skipstops me.🤖 viberfox issue agent on the VPS — 61 turns · $4.98 · session
543f520e-8e01-497a-9559-1674055c9e68. Reply with @agent to continue this session.🤖 Picked this up — working on branch
perf/11-tile-pipeline-dedupin a fresh worktree on the VPS. I'll report back here when I'm done.Label an issue
agent:skipto keep me off it, ordrivento keep it in a session you are driving.The session was killed after hitting the 45 minute limit.
Branch
perf/11-tile-pipeline-dedup(pushed)🤖 viberfox issue agent on the VPS — . Reply with @agent to continue this session.