Skip to content

Commit

Permalink
Share common CacheHandle (#7688)
Browse files Browse the repository at this point in the history
* Share common CacheHandle

* Changelog

* License update
  • Loading branch information
DzmitryFomchyn authored Jan 8, 2024
1 parent 5293d3a commit fba4c2d
Show file tree
Hide file tree
Showing 9 changed files with 70 additions and 74 deletions.
2 changes: 1 addition & 1 deletion LICENSE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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]

Expand Down
1 change: 1 addition & 0 deletions changelog/unreleased/bugfixes/7688.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Fixed a bug with multiple instances of cache which resulted in excessive memory consumption.
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,9 @@ internal open class MapboxNavigationBaseTest {
any(),
)
} returns mockk(relaxed = true)
every {
NavigatorLoader.createCacheHandle(any(), any(), any())
} returns mockk()

mockkObject(MapboxSDKCommon)
every {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -938,15 +938,9 @@ internal class MapboxNavigationTest : MapboxNavigationBaseTest() {
fun `verify tile config path`() {
threadController.cancelAllUICoroutines()
val slot = slot<TilesConfig>()
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()
Expand All @@ -960,15 +954,9 @@ internal class MapboxNavigationTest : MapboxNavigationBaseTest() {
fun `verify tile config dataset`() {
threadController.cancelAllUICoroutines()
val slot = slot<TilesConfig>()
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()
Expand Down Expand Up @@ -1252,15 +1240,9 @@ internal class MapboxNavigationTest : MapboxNavigationBaseTest() {
fun `verify tile config tilesVersion and isFallback on init`() {
threadController.cancelAllUICoroutines()
val slot = slot<TilesConfig>()
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(
Expand Down Expand Up @@ -1293,15 +1275,10 @@ internal class MapboxNavigationTest : MapboxNavigationBaseTest() {
mapboxNavigation = MapboxNavigation(navigationOptions, threadController)

val tileConfigSlot = slot<TilesConfig>()

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"
Expand Down Expand Up @@ -1333,15 +1310,10 @@ internal class MapboxNavigationTest : MapboxNavigationBaseTest() {
mapboxNavigation = MapboxNavigation(navigationOptions, threadController)

val tileConfigSlot = slot<TilesConfig>()

every {
navigator.recreate(
any(),
any(),
capture(tileConfigSlot),
any(),
any(),
)
} just Runs
NavigatorLoader.createCacheHandle(any(), capture(tileConfigSlot), any())
} returns mockk()

fallbackObserverSlot.captured.onCanReturnToLatest("")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -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,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down

0 comments on commit fba4c2d

Please sign in to comment.