From 4441df61f09ea4f620bdcb9c645dce8e341a02cc Mon Sep 17 00:00:00 2001 From: dzmitryfomchyn Date: Fri, 5 Jan 2024 04:05:00 +0100 Subject: [PATCH 1/3] Share common CacheHandle --- .../navigation/core/MapboxNavigation.kt | 36 ++++++++---- .../core/NavigationComponentProvider.kt | 6 +- .../core/MapboxNavigationBaseTest.kt | 3 + .../navigation/core/MapboxNavigationTest.kt | 58 +++++-------------- .../internal/MapboxNativeNavigator.kt | 5 +- .../internal/MapboxNativeNavigatorImpl.kt | 9 ++- .../navigator/internal/NavigatorLoader.kt | 24 +++++--- 7 files changed, 68 insertions(+), 73 deletions(-) diff --git a/libnavigation-core/src/main/java/com/mapbox/navigation/core/MapboxNavigation.kt b/libnavigation-core/src/main/java/com/mapbox/navigation/core/MapboxNavigation.kt index 87cc8df09d8..79ea03a1087 100644 --- a/libnavigation-core/src/main/java/com/mapbox/navigation/core/MapboxNavigation.kt +++ b/libnavigation-core/src/main/java/com/mapbox/navigation/core/MapboxNavigation.kt @@ -442,15 +442,26 @@ class MapboxNavigation @VisibleForTesting internal constructor( navigationOptions.deviceProfile, navigatorConfig, ) + + val tilesConfig = createTilesConfig( + isFallback = false, + tilesVersion = navigationOptions.routingTilesOptions.tilesVersion + ) + historyRecorderHandles = createHistoryRecorderHandles(config) + + val cacheHandle = NavigatorLoader.createCacheHandle( + config, + tilesConfig, + historyRecorderHandles.composite + ) + nativeRouter = NavigatorLoader.createNativeRouterInterface( - NavigatorLoader.createConfig(navigationOptions.deviceProfile, navigatorConfig), - createTilesConfig( - isFallback = false, - tilesVersion = navigationOptions.routingTilesOptions.tilesVersion - ), + cacheHandle, + config, historyRecorderHandles.composite, ) + val routeParsingManager = createRouteParsingManager( navigationOptions.longRoutesOptimisationOptions ) @@ -472,12 +483,9 @@ class MapboxNavigation @VisibleForTesting internal constructor( else -> LegacyNavigationRouterAdapter(LegacyRouterAdapter(result)) } navigator = NavigationComponentProvider.createNativeNavigator( + cacheHandle, config, historyRecorderHandles.composite, - createTilesConfig( - isFallback = false, - tilesVersion = navigationOptions.routingTilesOptions.tilesVersion - ), navigationOptions.accessToken ?: "", if (moduleRouter.isInternalImplementation()) { // We pass null to let NN know that default router is used and it can rely @@ -2076,10 +2084,18 @@ class MapboxNavigation @VisibleForTesting internal constructor( historyRecorderHandles = createHistoryRecorderHandles(config) mainJobController.scope.launch { + // TODO we should also recreate router and share CacheHandle + + val cacheHandle = NavigatorLoader.createCacheHandle( + config, + createTilesConfig(isFallback, tilesVersion), + historyRecorderHandles.composite + ) + navigator.recreate( + cacheHandle, config, historyRecorderHandles.composite, - createTilesConfig(isFallback, tilesVersion), navigationOptions.accessToken ?: "", if (moduleRouter.isInternalImplementation()) { nativeRouter diff --git a/libnavigation-core/src/main/java/com/mapbox/navigation/core/NavigationComponentProvider.kt b/libnavigation-core/src/main/java/com/mapbox/navigation/core/NavigationComponentProvider.kt index 3ac63a01e52..fed76a39c49 100644 --- a/libnavigation-core/src/main/java/com/mapbox/navigation/core/NavigationComponentProvider.kt +++ b/libnavigation-core/src/main/java/com/mapbox/navigation/core/NavigationComponentProvider.kt @@ -25,10 +25,10 @@ import com.mapbox.navigation.core.trip.session.eh.EHorizonSubscriptionManagerImp import com.mapbox.navigation.navigator.internal.MapboxNativeNavigator import com.mapbox.navigation.navigator.internal.MapboxNativeNavigatorImpl import com.mapbox.navigation.utils.internal.ThreadController +import com.mapbox.navigator.CacheHandle import com.mapbox.navigator.ConfigHandle import com.mapbox.navigator.HistoryRecorderHandle import com.mapbox.navigator.RouterInterface -import com.mapbox.navigator.TilesConfig import kotlinx.coroutines.CoroutineScope internal object NavigationComponentProvider { @@ -38,15 +38,15 @@ internal object NavigationComponentProvider { ): DirectionsSession = MapboxDirectionsSession(router) fun createNativeNavigator( + cacheHandle: CacheHandle, config: ConfigHandle, historyRecorderComposite: HistoryRecorderHandle?, - tilesConfig: TilesConfig, accessToken: String, router: RouterInterface?, ): MapboxNativeNavigator = MapboxNativeNavigatorImpl.create( + cacheHandle, config, historyRecorderComposite, - tilesConfig, accessToken, router, ) diff --git a/libnavigation-core/src/test/java/com/mapbox/navigation/core/MapboxNavigationBaseTest.kt b/libnavigation-core/src/test/java/com/mapbox/navigation/core/MapboxNavigationBaseTest.kt index 0849a0fe05e..1b462ecc4bb 100644 --- a/libnavigation-core/src/test/java/com/mapbox/navigation/core/MapboxNavigationBaseTest.kt +++ b/libnavigation-core/src/test/java/com/mapbox/navigation/core/MapboxNavigationBaseTest.kt @@ -151,6 +151,9 @@ internal open class MapboxNavigationBaseTest { any(), ) } returns mockk(relaxed = true) + every { + NavigatorLoader.createCacheHandle(any(), any(), any()) + } returns mockk() mockkObject(MapboxSDKCommon) every { diff --git a/libnavigation-core/src/test/java/com/mapbox/navigation/core/MapboxNavigationTest.kt b/libnavigation-core/src/test/java/com/mapbox/navigation/core/MapboxNavigationTest.kt index a995efaef7e..9f5d3019dc3 100644 --- a/libnavigation-core/src/test/java/com/mapbox/navigation/core/MapboxNavigationTest.kt +++ b/libnavigation-core/src/test/java/com/mapbox/navigation/core/MapboxNavigationTest.kt @@ -938,15 +938,9 @@ internal class MapboxNavigationTest : MapboxNavigationBaseTest() { fun `verify tile config path`() { threadController.cancelAllUICoroutines() val slot = slot() - every { - NavigationComponentProvider.createNativeNavigator( - any(), - any(), - capture(slot), - any(), - any(), - ) - } returns navigator + + every { NavigatorLoader.createCacheHandle(any(), capture(slot), any()) } returns mockk() + val options = navigationOptions.toBuilder() .routingTilesOptions(RoutingTilesOptions.Builder().build()) .build() @@ -960,15 +954,9 @@ internal class MapboxNavigationTest : MapboxNavigationBaseTest() { fun `verify tile config dataset`() { threadController.cancelAllUICoroutines() val slot = slot() - every { - NavigationComponentProvider.createNativeNavigator( - any(), - any(), - capture(slot), - any(), - any(), - ) - } returns navigator + + every { NavigatorLoader.createCacheHandle(any(), capture(slot), any()) } returns mockk() + val options = navigationOptions.toBuilder() .routingTilesOptions( RoutingTilesOptions.Builder() @@ -1252,15 +1240,9 @@ internal class MapboxNavigationTest : MapboxNavigationBaseTest() { fun `verify tile config tilesVersion and isFallback on init`() { threadController.cancelAllUICoroutines() val slot = slot() - every { - NavigationComponentProvider.createNativeNavigator( - any(), - any(), - capture(slot), - any(), - any(), - ) - } returns navigator + + every { NavigatorLoader.createCacheHandle(any(), capture(slot), any()) } returns mockk() + val tilesVersion = "tilesVersion" val options = navigationOptions.toBuilder() .routingTilesOptions( @@ -1293,15 +1275,10 @@ internal class MapboxNavigationTest : MapboxNavigationBaseTest() { mapboxNavigation = MapboxNavigation(navigationOptions, threadController) val tileConfigSlot = slot() + every { - navigator.recreate( - any(), - any(), - capture(tileConfigSlot), - any(), - any(), - ) - } just Runs + NavigatorLoader.createCacheHandle(any(), capture(tileConfigSlot), any()) + } returns mockk() val tilesVersion = "tilesVersion" val latestTilesVersion = "latestTilesVersion" @@ -1333,15 +1310,10 @@ internal class MapboxNavigationTest : MapboxNavigationBaseTest() { mapboxNavigation = MapboxNavigation(navigationOptions, threadController) val tileConfigSlot = slot() + every { - navigator.recreate( - any(), - any(), - capture(tileConfigSlot), - any(), - any(), - ) - } just Runs + NavigatorLoader.createCacheHandle(any(), capture(tileConfigSlot), any()) + } returns mockk() fallbackObserverSlot.captured.onCanReturnToLatest("") diff --git a/libnavigator/src/main/java/com/mapbox/navigation/navigator/internal/MapboxNativeNavigator.kt b/libnavigator/src/main/java/com/mapbox/navigation/navigator/internal/MapboxNativeNavigator.kt index 95beec77f0f..043bf025b11 100644 --- a/libnavigator/src/main/java/com/mapbox/navigation/navigator/internal/MapboxNativeNavigator.kt +++ b/libnavigator/src/main/java/com/mapbox/navigation/navigator/internal/MapboxNativeNavigator.kt @@ -27,7 +27,6 @@ import com.mapbox.navigator.RouteAlternativesControllerInterface import com.mapbox.navigator.RouterInterface import com.mapbox.navigator.SetRoutesReason import com.mapbox.navigator.SetRoutesResult -import com.mapbox.navigator.TilesConfig /** * Provides API to work with native Navigator class. Exposed for internal usage only. @@ -38,9 +37,9 @@ interface MapboxNativeNavigator { * Initialize the navigator with a device profile */ fun create( + cacheHandle: CacheHandle, config: ConfigHandle, historyRecorderComposite: HistoryRecorderHandle?, - tilesConfig: TilesConfig, accessToken: String, router: RouterInterface?, ): MapboxNativeNavigator @@ -49,9 +48,9 @@ interface MapboxNativeNavigator { * Reinitialize the navigator with a device profile */ fun recreate( + cacheHandle: CacheHandle, config: ConfigHandle, historyRecorderComposite: HistoryRecorderHandle?, - tilesConfig: TilesConfig, accessToken: String, router: RouterInterface, ) diff --git a/libnavigator/src/main/java/com/mapbox/navigation/navigator/internal/MapboxNativeNavigatorImpl.kt b/libnavigator/src/main/java/com/mapbox/navigation/navigator/internal/MapboxNativeNavigatorImpl.kt index 8269d3fa3eb..8fc34cc6959 100644 --- a/libnavigator/src/main/java/com/mapbox/navigation/navigator/internal/MapboxNativeNavigatorImpl.kt +++ b/libnavigator/src/main/java/com/mapbox/navigation/navigator/internal/MapboxNativeNavigatorImpl.kt @@ -43,7 +43,6 @@ import com.mapbox.navigator.RouterInterface import com.mapbox.navigator.SetRoutesParams import com.mapbox.navigator.SetRoutesReason import com.mapbox.navigator.SetRoutesResult -import com.mapbox.navigator.TilesConfig import kotlinx.coroutines.suspendCancellableCoroutine import kotlinx.coroutines.withContext import java.util.concurrent.CopyOnWriteArraySet @@ -78,18 +77,18 @@ object MapboxNativeNavigatorImpl : MapboxNativeNavigator { * functions within [MapboxNativeNavigatorImpl] */ override fun create( + cacheHandle: CacheHandle, config: ConfigHandle, historyRecorderComposite: HistoryRecorderHandle?, - tilesConfig: TilesConfig, accessToken: String, router: RouterInterface?, ): MapboxNativeNavigator { navigator?.shutdown() val nativeComponents = NavigatorLoader.createNavigator( + cacheHandle, config, historyRecorderComposite, - tilesConfig, router, ) navigator = nativeComponents.navigator @@ -107,14 +106,14 @@ object MapboxNativeNavigatorImpl : MapboxNativeNavigator { * Recreate native objects and notify listeners. */ override fun recreate( + cacheHandle: CacheHandle, config: ConfigHandle, historyRecorderComposite: HistoryRecorderHandle?, - tilesConfig: TilesConfig, accessToken: String, router: RouterInterface ) { val storeNavSessionState = navigator!!.storeNavigationSession() - create(config, historyRecorderComposite, tilesConfig, accessToken, router) + create(cacheHandle, config, historyRecorderComposite, accessToken, router) navigator!!.restoreNavigationSession(storeNavSessionState) nativeNavigatorRecreationObservers.forEach { it.onNativeNavigatorRecreated() diff --git a/libnavigator/src/main/java/com/mapbox/navigation/navigator/internal/NavigatorLoader.kt b/libnavigator/src/main/java/com/mapbox/navigation/navigator/internal/NavigatorLoader.kt index 57454a80bf6..f921045997c 100644 --- a/libnavigator/src/main/java/com/mapbox/navigation/navigator/internal/NavigatorLoader.kt +++ b/libnavigator/src/main/java/com/mapbox/navigation/navigator/internal/NavigatorLoader.kt @@ -49,39 +49,45 @@ object NavigatorLoader { } internal fun createNavigator( + cacheHandle: CacheHandle, config: ConfigHandle, historyRecorderComposite: HistoryRecorderHandle?, - tilesConfig: TilesConfig, router: RouterInterface?, ): NativeComponents { - val cache = CacheFactory.build(tilesConfig, config, historyRecorderComposite) val navigator = Navigator( config, - cache, + cacheHandle, historyRecorderComposite, router, ) - val graphAccessor = GraphAccessor(cache) - val roadObjectMatcher = RoadObjectMatcher(cache) + val graphAccessor = GraphAccessor(cacheHandle) + val roadObjectMatcher = RoadObjectMatcher(cacheHandle) return NativeComponents( navigator, graphAccessor, - cache, + cacheHandle, roadObjectMatcher, navigator.routeAlternativesController, ) } - fun createNativeRouterInterface( + fun createCacheHandle( config: ConfigHandle, tilesConfig: TilesConfig, historyRecorder: HistoryRecorderHandle?, + ): CacheHandle { + return CacheFactory.build(tilesConfig, config, historyRecorder) + } + + fun createNativeRouterInterface( + cacheHandle: CacheHandle, + config: ConfigHandle, + historyRecorder: HistoryRecorderHandle?, ): RouterInterface { - val cache = CacheFactory.build(tilesConfig, config, historyRecorder) return RouterFactory.build( RouterType.HYBRID, - cache, + cacheHandle, config, historyRecorder, ) From 8a219501a58b5e3d0b8d4beffc0790a01a5f4c2d Mon Sep 17 00:00:00 2001 From: dzmitryfomchyn Date: Mon, 8 Jan 2024 12:33:53 +0100 Subject: [PATCH 2/3] Update license --- LICENSE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/LICENSE.md b/LICENSE.md index e952855ee3f..ac2c7201275 100644 --- a/LICENSE.md +++ b/LICENSE.md @@ -3,7 +3,7 @@ Mapbox Navigation for Android version 2.0 Mapbox Navigation Android SDK -Copyright ©2022 - 2023 Mapbox, Inc. All rights reserved. +Copyright ©2022 - 2024 Mapbox, Inc. All rights reserved. The software and files in this repository (collectively, "Software") are licensed under the Mapbox TOS for use only with the relevant Mapbox product(s) listed at www.mapbox.com/pricing. This license allows developers with a current active Mapbox account to use and modify the authorized portions of the Software as needed for use only with the relevant Mapbox product(s) through their Mapbox account in accordance with the Mapbox TOS. This license terminates automatically if a developer no longer has a Mapbox account in good standing or breaches the Mapbox TOS. For the license terms, please see the Mapbox TOS at https://www.mapbox.com/legal/tos/ which incorporates the Mapbox Product Terms at www.mapbox.com/legal/service-terms. If this Software is a SDK, modifications that change or interfere with marked portions of the code related to billing, accounting, or data collection are not authorized and the SDK sends limited de-identified location and usage data which is used in accordance with the Mapbox TOS. [Updated 2023-04] From 40f4149a928bad6d4ffd04524539082bb64d2fa3 Mon Sep 17 00:00:00 2001 From: dzmitryfomchyn Date: Mon, 8 Jan 2024 12:37:15 +0100 Subject: [PATCH 3/3] Changelog --- changelog/unreleased/bugfixes/7692.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog/unreleased/bugfixes/7692.md diff --git a/changelog/unreleased/bugfixes/7692.md b/changelog/unreleased/bugfixes/7692.md new file mode 100644 index 00000000000..0c130f6fa9f --- /dev/null +++ b/changelog/unreleased/bugfixes/7692.md @@ -0,0 +1 @@ +- Fixed a bug with multiple instances of cache which resulted in excessive memory consumption. \ No newline at end of file