From bb7ac50226df8414656568c6a7f6d2c9f308e413 Mon Sep 17 00:00:00 2001 From: Masashi Katsumata Date: Sat, 7 Nov 2026 16:20:26 -0800 Subject: [PATCH 1/2] Let a state drop a destroyed controller; clear overlay entities on destroy The state outlives the map: apps keep it (the example app keeps one per provider), and it held the last controller after the map was torn down, and through it every overlay entity of that map. detachController(controller) lets go of it, unless another controller has been attached since. Overlay controllers' destroy() now clears their managers, so a destroyed controller that is still referenced keeps no entities. An add() in flight when that happens stops instead of tripping over the missing entity. --- api/android-sdk-core.api | 1 + .../core/circle/CircleController.kt | 7 +++-- .../core/groundimage/GroundImageController.kt | 7 +++-- .../com/mapconductor/core/map/MapViewState.kt | 17 ++++++++++++ .../core/polygon/PolygonController.kt | 7 +++-- .../core/polyline/PolylineController.kt | 7 +++-- .../core/raster/RasterLayerController.kt | 4 ++- .../core/map/MapViewStateBaseTest.kt | 27 +++++++++++++++++++ 8 files changed, 68 insertions(+), 9 deletions(-) diff --git a/api/android-sdk-core.api b/api/android-sdk-core.api index e1ffa9d..0aef326 100644 --- a/api/android-sdk-core.api +++ b/api/android-sdk-core.api @@ -1095,6 +1095,7 @@ public abstract class com.mapconductor.core.map.MapViewState( data.forEach { state -> if (previous.contains(state.id)) { - val prevEntity = circleManager.getEntity(state.id)!! + // Gone only if destroy() cleared the manager meanwhile; nothing left to update. + val prevEntity = circleManager.getEntity(state.id) ?: return@withPermit if (state.fingerPrint() == prevEntity.fingerPrint) { // 描画結果が不変なら renderer を呼ばず最新の state だけ採用する // （react-sdk と同じ。composition ごとに全オーバーレイへ onChange を @@ -158,7 +159,9 @@ abstract class CircleController( override fun find(position: GeoPointInterface): CircleEntityInterface? = circleManager.find(position) override fun destroy() { - // No native resources to clean up for circles + // A destroyed controller can still be referenced (by its map controller, + // or a state that has not let go yet); without this it keeps every entity. + circleManager.clear() } override fun has(id: String): Boolean = circleManager.hasEntity(id) diff --git a/src/main/java/com/mapconductor/core/groundimage/GroundImageController.kt b/src/main/java/com/mapconductor/core/groundimage/GroundImageController.kt index b51818a..1c5cbc9 100644 --- a/src/main/java/com/mapconductor/core/groundimage/GroundImageController.kt +++ b/src/main/java/com/mapconductor/core/groundimage/GroundImageController.kt @@ -36,7 +36,8 @@ abstract class GroundImageController( data.forEach { state -> if (previous.contains(state.id)) { - val prevEntity = groundImageManager.getEntity(state.id)!! + // Gone only if destroy() cleared the manager meanwhile; nothing left to update. + val prevEntity = groundImageManager.getEntity(state.id) ?: return@withPermit if (state.fingerPrint() == prevEntity.fingerPrint) { // 描画結果が不変なら renderer を呼ばず最新の state だけ採用する（react-sdk と同じ）。 groundImageManager @@ -157,7 +158,9 @@ abstract class GroundImageController( .find(position) override fun destroy() { - // No native resources to clean up + // A destroyed controller can still be referenced (by its map controller, + // or a state that has not let go yet); without this it keeps every entity. + groundImageManager.clear() } override fun has(id: String): Boolean = groundImageManager.hasEntity(id) diff --git a/src/main/java/com/mapconductor/core/map/MapViewState.kt b/src/main/java/com/mapconductor/core/map/MapViewState.kt index bd6e7e7..9ce07ed 100644 --- a/src/main/java/com/mapconductor/core/map/MapViewState.kt +++ b/src/main/java/com/mapconductor/core/map/MapViewState.kt @@ -148,6 +148,23 @@ abstract class MapViewState( this.attachedMapController = null } + /** + * 破棄した [controller] を手放す。地図が壊されたあとで呼ぶ。 + * + * state は地図より長生きする（アプリが保持し、プロバイダを切り替えても残る）。 + * ここで手放さないと、state が最後のコントローラ（とその全オーバーレイ）を + * 抱えたままになる。Compose では `MapViewBase` が地図をコンポジションから + * 外したときに呼ぶので、プロバイダが呼ぶ必要はない。 + * + * その間に別のコントローラが接続されていれば何もしない（新しい地図を + * 古い地図の後始末で切り離さないため）。 + */ + fun detachController(controller: MapViewControllerInterface) { + if (this.attachedMapController === controller) { + this.attachedMapController = null + } + } + /** * 地図から通知された現在のカメラを保持する（地図を動かさない）。 * diff --git a/src/main/java/com/mapconductor/core/polygon/PolygonController.kt b/src/main/java/com/mapconductor/core/polygon/PolygonController.kt index 96eb44f..c9ad5c9 100644 --- a/src/main/java/com/mapconductor/core/polygon/PolygonController.kt +++ b/src/main/java/com/mapconductor/core/polygon/PolygonController.kt @@ -35,7 +35,8 @@ abstract class PolygonController( data.forEach { state -> if (previous.contains(state.id)) { - val prevEntity = polygonManager.getEntity(state.id)!! + // Gone only if destroy() cleared the manager meanwhile; nothing left to update. + val prevEntity = polygonManager.getEntity(state.id) ?: return@withPermit if (state.fingerPrint() == prevEntity.fingerPrint) { // 描画結果が不変なら renderer を呼ばず最新の state だけ採用する（react-sdk と同じ）。 polygonManager.registerEntity(PolygonEntity(state = state, polygon = prevEntity.polygon)) @@ -162,7 +163,9 @@ abstract class PolygonController( .find(position) override fun destroy() { - // No native resources to clean up for polygons + // A destroyed controller can still be referenced (by its map controller, + // or a state that has not let go yet); without this it keeps every entity. + polygonManager.clear() } override fun has(id: String): Boolean = polygonManager.hasEntity(id) diff --git a/src/main/java/com/mapconductor/core/polyline/PolylineController.kt b/src/main/java/com/mapconductor/core/polyline/PolylineController.kt index d5080cd..361cda6 100644 --- a/src/main/java/com/mapconductor/core/polyline/PolylineController.kt +++ b/src/main/java/com/mapconductor/core/polyline/PolylineController.kt @@ -39,7 +39,8 @@ abstract class PolylineController( data.forEach { state -> if (previous.contains(state.id)) { - val prevEntity = polylineManager.getEntity(state.id)!! + // Gone only if destroy() cleared the manager meanwhile; nothing left to update. + val prevEntity = polylineManager.getEntity(state.id) ?: return@withPermit if (state.fingerPrint() == prevEntity.fingerPrint) { // 描画結果が不変なら renderer を呼ばず最新の state だけ採用する（react-sdk と同じ）。 polylineManager.registerEntity(PolylineEntity(state = state, polyline = prevEntity.polyline)) @@ -169,7 +170,9 @@ abstract class PolylineController( } override fun destroy() { - // No native resources to clean up for polylines + // A destroyed controller can still be referenced (by its map controller, + // or a state that has not let go yet); without this it keeps every entity. + polylineManager.clear() } override fun has(id: String): Boolean = polylineManager.hasEntity(id) diff --git a/src/main/java/com/mapconductor/core/raster/RasterLayerController.kt b/src/main/java/com/mapconductor/core/raster/RasterLayerController.kt index 4b3d831..f6b3631 100644 --- a/src/main/java/com/mapconductor/core/raster/RasterLayerController.kt +++ b/src/main/java/com/mapconductor/core/raster/RasterLayerController.kt @@ -282,7 +282,9 @@ abstract class RasterLayerController( } override fun destroy() { - // No native resources to clean up + // A destroyed controller can still be referenced (by its map controller, + // or a state that has not let go yet); without this it keeps every entity. + rasterLayerManager.clear() } override fun has(id: String): Boolean = rasterLayerManager.hasEntity(id) diff --git a/src/test/java/com/mapconductor/core/map/MapViewStateBaseTest.kt b/src/test/java/com/mapconductor/core/map/MapViewStateBaseTest.kt index 19a5e8e..73d519c 100644 --- a/src/test/java/com/mapconductor/core/map/MapViewStateBaseTest.kt +++ b/src/test/java/com/mapconductor/core/map/MapViewStateBaseTest.kt @@ -258,4 +258,31 @@ class MapViewStateBaseTest { assertTrue(controller.moved.isEmpty()) assertNull(state.getMapViewHolder()) } + + @Test + fun `破棄したコントローラを渡すと手放す`() { + // アプリが state を持ち続けても、壊した地図のコントローラ（と全オーバーレイ）を + // 抱えたままにしない。 + val controller = FakeController() + val state = TestState().apply { attach(controller) } + + state.detachController(controller) + + assertNull(state.getMapViewHolder()) + } + + @Test + fun `別のコントローラが接続済みなら古いものを渡しても外さない`() { + val old = FakeController() + val current = FakeController() + val state = TestState().apply { attach(old) } + state.attach(current) + + state.detachController(old) + + assertSame(current.holder, state.getMapViewHolder()) + current.moved.clear() + state.moveCameraTo(cameraAt(1.0, 2.0), durationMillis = 0) + assertEquals(1, current.moved.size) + } } From 6bb8f16aaa307a16b3a24cc646b492dca98e8815 Mon Sep 17 00:00:00 2001 From: Masashi Katsumata Date: Sat, 7 Nov 2026 16:36:32 -0800 Subject: [PATCH 2/2] Pool tile scratch buffers instead of keeping one per thread TilePngEncoder and MarkerTileRenderer kept their scratch (a ~1.2 MB direct buffer at this tablet's tile size, an output stream) in a ThreadLocal. Tiles are drawn on the map SDK's threads, and Google Maps starts a pool of three "androidmapsapi-tileoverlay" threads for every map and never shuts it down, so each destroyed Google map left three live threads with a buffer each: about 1.7 MB of Java heap per provider switch that nothing could free (heap dumps after GC, MapLibre <-> Google with Boston parcels). ScratchPool keeps at most a few of them (about one per core) whatever the threads do. The threads themselves are the SDK's and remain. --- api/android-sdk-core.api | 12 +++++ .../core/marker/MarkerTileRenderer.kt | 29 ++++++----- .../core/tileserver/ScratchPool.kt | 49 +++++++++++++++++++ .../core/tileserver/TilePngEncoder.kt | 31 ++++++------ .../core/tileserver/ScratchPoolTest.kt | 42 ++++++++++++++++ 5 files changed, 136 insertions(+), 27 deletions(-) create mode 100644 src/main/java/com/mapconductor/core/tileserver/ScratchPool.kt create mode 100644 src/test/java/com/mapconductor/core/tileserver/ScratchPoolTest.kt diff --git a/api/android-sdk-core.api b/api/android-sdk-core.api index 0aef326..bbdf101 100644 --- a/api/android-sdk-core.api +++ b/api/android-sdk-core.api @@ -2929,6 +2929,18 @@ public final class com.mapconductor.core.tileserver.LocalTileServer$TileOutcome$ public final class com.mapconductor.core.tileserver.LocalTileServerKt { public static final com.mapconductor.core.tileserver.TileCoordinate parseTileCoordinate(java.lang.String); } +public final class com.mapconductor.core.tileserver.ScratchPool { + public static final com.mapconductor.core.tileserver.ScratchPool$Companion Companion; + public com.mapconductor.core.tileserver.ScratchPool(int); + public com.mapconductor.core.tileserver.ScratchPool(int, int, kotlin.jvm.internal.DefaultConstructorMarker); + public final T acquire(kotlin.jvm.functions.Function1); + public final boolean release(T); + public final int getSize(); + public com.mapconductor.core.tileserver.ScratchPool(); +} +public final class com.mapconductor.core.tileserver.ScratchPool$Companion { + public final int getDEFAULT_CAPACITY(); +} public final class com.mapconductor.core.tileserver.TileCoordinate { public com.mapconductor.core.tileserver.TileCoordinate(int, int); public final int getY(); diff --git a/src/main/java/com/mapconductor/core/marker/MarkerTileRenderer.kt b/src/main/java/com/mapconductor/core/marker/MarkerTileRenderer.kt index 0357e52..f664be5 100644 --- a/src/main/java/com/mapconductor/core/marker/MarkerTileRenderer.kt +++ b/src/main/java/com/mapconductor/core/marker/MarkerTileRenderer.kt @@ -6,6 +6,7 @@ import androidx.core.graphics.createBitmap import com.mapconductor.core.ResourceProvider import com.mapconductor.core.features.GeoPoint import com.mapconductor.core.features.GeoRectBounds +import com.mapconductor.core.tileserver.ScratchPool import com.mapconductor.core.tileserver.TilePngEncoder import com.mapconductor.core.tileserver.TileProviderInterface import com.mapconductor.core.tileserver.TileRequest @@ -686,10 +687,10 @@ class MarkerTileRenderer( // ByteArrayOutputStream starts at 32 bytes and doubles its backing array on every // compress() call until it reaches the tile's steady-state size (often hundreds of KB // for dense tiles), so a new instance per call re-triggers that regrowth every time. - // Keeping one per thread (reset, not recreated, before each use) lets the backing - // array settle at its steady-state size and be reused across tiles. - private val tileByteStream = - ThreadLocal.withInitial { ByteArrayOutputStream(16 * 1024) } + // Keeping a few (reset, not recreated, before each use) lets the backing array + // settle at its steady-state size and be reused across tiles. A pool, not one per + // thread: map SDK threads that outlive their map would keep theirs (ScratchPool). + private val tileByteStreams = ScratchPool() private fun bitmapToByteArray(bitmap: Bitmap): ByteArray { // Rust first: it is 4-6x faster than Bitmap.compress on the tiles this @@ -698,15 +699,17 @@ class MarkerTileRenderer( // declined the bitmap, and the platform encoder takes over. TilePngEncoder.encode(bitmap)?.let { return it } - // ThreadLocal.get() is a Java generic method, so Kotlin sees its return type as - // nullable even though withInitial() guarantees a value; !! is safe here. - val outputStream = tileByteStream.get()!! - outputStream.reset() - bitmap.compress(Bitmap.CompressFormat.PNG, 100, outputStream) - // toByteArray() still copies out a right-sized array: callers cache this array or - // hand it to a socket write, both of which need a stable array that isn't mutated - // by the next renderTile() call on this thread. - return outputStream.toByteArray() + val outputStream = tileByteStreams.acquire() ?: ByteArrayOutputStream(16 * 1024) + try { + outputStream.reset() + bitmap.compress(Bitmap.CompressFormat.PNG, 100, outputStream) + // toByteArray() still copies out a right-sized array: callers cache this array or + // hand it to a socket write, both of which need a stable array that isn't mutated + // by the next renderTile() call. + return outputStream.toByteArray() + } finally { + tileByteStreams.release(outputStream) + } } private fun acquireBitmap(size: Int): Bitmap { diff --git a/src/main/java/com/mapconductor/core/tileserver/ScratchPool.kt b/src/main/java/com/mapconductor/core/tileserver/ScratchPool.kt new file mode 100644 index 0000000..dcfd778 --- /dev/null +++ b/src/main/java/com/mapconductor/core/tileserver/ScratchPool.kt @@ -0,0 +1,49 @@ +package com.mapconductor.core.tileserver + +/** + * A few reusable scratch objects (pixel buffers, bitmaps, output streams) for + * code that renders tiles on threads it does not own. + * + * Use this instead of a `ThreadLocal`. Tile callbacks run on the map SDK's + * threads, and some SDKs start new ones for every map and never stop them: + * Google Maps keeps an `androidmapsapi-tileoverlay` pool of three live threads + * per map it ever created. A thread-local scratch pins one object on each such + * thread for good, so every map switch added a few megabytes that nothing + * could free. A pool holds at most [capacity] objects whatever the threads do. + * + * [acquire] hands out a pooled object or null (the caller makes a fresh one); + * [release] takes it back, or drops it when the pool is full. + */ +internal class ScratchPool( + private val capacity: Int = DEFAULT_CAPACITY, +) { + private val items = ArrayDeque(capacity) + + /** A pooled object that [accept] takes, or null. Others are dropped. */ + fun acquire(accept: (T) -> Boolean = { true }): T? { + synchronized(items) { + while (items.isNotEmpty()) { + val item = items.removeLast() + if (accept(item)) return item + } + } + return null + } + + /** Returns [item]. False if the pool was full and did not keep it. */ + fun release(item: T): Boolean = + synchronized(items) { + if (items.size >= capacity) return false + items.addLast(item) + true + } + + /** Pooled objects right now. For tests. */ + val size: Int + get() = synchronized(items) { items.size } + + companion object { + /** About how many tiles are drawn at the same time. */ + val DEFAULT_CAPACITY: Int = Runtime.getRuntime().availableProcessors().coerceIn(2, 8) + } +} diff --git a/src/main/java/com/mapconductor/core/tileserver/TilePngEncoder.kt b/src/main/java/com/mapconductor/core/tileserver/TilePngEncoder.kt index 7c09b6a..4bea4ca 100644 --- a/src/main/java/com/mapconductor/core/tileserver/TilePngEncoder.kt +++ b/src/main/java/com/mapconductor/core/tileserver/TilePngEncoder.kt @@ -23,13 +23,15 @@ import android.graphics.Bitmap */ object TilePngEncoder { /** - * Scratch pixel buffer, one per calling thread. + * Scratch pixel buffers, shared by the calling threads. * - * Tile rendering runs concurrently across the tile server's threads, and a - * tile at this SDK's densities is around 7 MB of RGBA — allocating that per - * call would cost more than the encoding it is meant to speed up. + * Tile rendering runs concurrently across several threads, and a tile at + * this SDK's densities is around 7 MB of RGBA — allocating that per call + * would cost more than the encoding it is meant to speed up. Not one per + * thread: the callers include map SDK threads that outlive their map (see + * [ScratchPool]), and each would keep its buffer for good. */ - private val scratch = ThreadLocal() + private val scratch = ScratchPool() /** False where the native library is missing for this device's ABI. */ val isAvailable: Boolean = @@ -55,16 +57,17 @@ object TilePngEncoder { if (width <= 0 || height <= 0) return null val needed = width * height * 4 - var buffer = scratch.get() - if (buffer == null || buffer.capacity() < needed) { - buffer = ByteBuffer.allocateDirect(needed).order(ByteOrder.nativeOrder()) - scratch.set(buffer) + val buffer = + scratch.acquire { it.capacity() >= needed } + ?: ByteBuffer.allocateDirect(needed).order(ByteOrder.nativeOrder()) + try { + buffer.clear() + bitmap.copyPixelsToBuffer(buffer) + buffer.rewind() + return encode(buffer, width, height, premultiplied = bitmap.isPremultiplied) + } finally { + scratch.release(buffer) } - buffer.clear() - bitmap.copyPixelsToBuffer(buffer) - buffer.rewind() - - return encode(buffer, width, height, premultiplied = bitmap.isPremultiplied) } /** diff --git a/src/test/java/com/mapconductor/core/tileserver/ScratchPoolTest.kt b/src/test/java/com/mapconductor/core/tileserver/ScratchPoolTest.kt new file mode 100644 index 0000000..61d2e5b --- /dev/null +++ b/src/test/java/com/mapconductor/core/tileserver/ScratchPoolTest.kt @@ -0,0 +1,42 @@ +package com.mapconductor.core.tileserver + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertSame +import org.junit.Assert.assertTrue +import org.junit.Test + +class ScratchPoolTest { + @Test + fun `holds no more than its capacity, however many threads used it`() { + // A thread-local kept one per thread, and map SDK threads that never stop + // (Google's tile-overlay pool, three per map) kept theirs for good. + val pool = ScratchPool(capacity = 2) + val released = (1..10).map { pool.release(ByteArray(1)) } + + assertEquals(listOf(true, true) + List(8) { false }, released) + assertEquals(2, pool.size) + } + + @Test + fun `hands back what was released`() { + val pool = ScratchPool(capacity = 2) + val item = ByteArray(4) + pool.release(item) + + assertSame(item, pool.acquire()) + assertNull(pool.acquire()) + } + + @Test + fun `drops pooled objects the caller cannot use`() { + val pool = ScratchPool(capacity = 2) + pool.release(ByteArray(4)) + + assertNull(pool.acquire { it.size >= 8 }) + assertEquals(0, pool.size) + assertTrue(pool.release(ByteArray(8))) + assertFalse(pool.acquire { it.size >= 8 } == null) + } +}