Bound MapTiles tile accumulation with deferred LRU eviction — fix zoom/pan OOM (#98)#115
Merged
Conversation
LRU eviction cap in MapTiles::paint() to bound tiles_ growth on OSM/WMTS basemap pan
MapTiles::paint() minted a Tile per newly-visible address and only hid off-screen tiles; the sole deletion path was setLayout(), reached only by the radar overlay's ~5-min refresh. The OSM/WMTS basemap never refreshes, so tiles_ grew for every tile area visited during a pan/zoom session until the process was OOM-killed — the silent crash reported in #98. Cap tiles_ at max(256, 4 * visible) and evict the least-recently-visible off-screen tiles. Eviction is DEFERRED off paint(): deleting a Tile (a QGraphicsObject scene child) mid-paint is a use-after-free risk, so paint() only records a per-tile last-visible generation and, when over the cap, queues evictIfNeeded() onto the event loop (debounced via eviction_pending_). evictIfNeeded() recomputes the cap from the live visible count, guards on tile->isVisible() so a re-shown tile is never evicted, and deletes oldest last-visible-generation first until tiles_ <= cap. setLayout() clears the bookkeeping; paint_generation_ stays monotonic. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add test_map_tiles_eviction.cpp: a multi-zoom-level layout (1x1 front + 20x20 deep) so paint() mints deep tiles on demand — a single-zoom layout would not, since setLayout() pre-seeds the whole front grid at construction. paint() is driven directly with a hand-built world transform (uniform scale with a web-mercator y-flip) chosen so every index lands on exact integers; a one-tile viewport is panned across the whole grid and processEvents() is called after each paint so the deferred evictIfNeeded() runs. Asserts tileChildCount() stays <= the 256 cap. A second test confirms the deep tiles really are minted by paint(). Verified non-vacuous: with eviction stubbed out, tiles_ reaches 401 and the bound assertion fails. Register the test in CMakeLists.txt mirroring test_map_tiles_refresh, update that test's SCOPE comment to point at this new within-cycle coverage, and sync plan.md to the as-built deferred-eviction design and resolved questions. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #98
Summary
CAMP silently died (OOM-kill) mid-survey while the operator zoomed/panned the map (recurring across 06-15/16/17). Corrected root cause (the issue's "#96 GDAL leak" premise was refuted during investigation):
MapTiles::paint()creates anew Tileper newly-visible tile and onlysetVisible(false)for off-screen ones — the only deletion was insetLayout(). The radar overlay self-bounds (5-min refresh →setLayout), but the OSM/WMTS basemap has no refresh, sotiles_grew unbounded across a pan/zoom session → RSS climbed → OOM-kill (silent, no stacktrace). (RasterLayerhas no viewport handler, so zoom/pan never re-invokes the #96 path — #96 was a real but separate leak, fixed in #114.)Fix — deferred LRU eviction bounds
tiles_max(256, 4×visible)(kTileEvictionMinCap=256,kTileEvictionMultiplier=4). OSM tiles decode to ~256×256 ARGB ≈ 256 KB, so the 256 floor ≈ ~64 MB tile buffer — documented at the definition site.QGraphicsObjectscene child insidepaint()is a use-after-free risk).paint()records a per-tile last-visible generation and, when over cap, queuesevictIfNeeded()viaQt::QueuedConnection(debounced by aneviction_pending_flag so only one is queued).evictIfNeeded()runs in the event loop: deletes oldest-by-generation non-visible tiles (guarded onisVisible()at eviction time, so a currently-visible tile is never evicted) untiltiles_.size() ≤ cap.cap ≥ visible_countguarantees enough candidates.setLayout()clears the generation map;paint_generation_stays monotonic.Test
test_map_tiles_eviction.cpp— a multi-zoom layout (1×1 front + 20×20 deep; single-zoom wouldn't reproduce the bug sincesetLayout()pre-seeds the front grid). Pans a 1-tile viewport across all 400 deep positions withprocessEvents()after each paint (eviction is deferred), assertingtileChildCount()stays ≤ cap throughout. Non-vacuity verified empirically: with eviction stubbed out, the sweep drivestiles_to 401 and the test fails (401 vs 256). A second test confirms the deep tiles are actually minted bypaint().105 tests, 0 failures (2 pre-existing GL-gated skips). Deep review-code, 0 must-fix.
Consequences
Updated the now-stale
onRefreshTimercomment ("paint() only hides, never deletes") and thetest_map_tiles_refresh.cppscope comment to point at the new eviction coverage.Authored-By:
Claude Code AgentModel:
Claude Opus 4.8