From 286310c7fe203144f7dfcb238559f72e923b4819 Mon Sep 17 00:00:00 2001 From: Seth Bourget Date: Tue, 25 Jun 2024 10:31:50 -0700 Subject: [PATCH] Using semaphore rather than synchronizing on generic object. --- .../examples/mincompile/DropInUIActivity.kt | 2 - .../internal/route/RouteCompatibilityCache.kt | 43 ++++++++++++------- .../route/RouteCompatibilityCacheTest.kt | 23 ++++++++++ 3 files changed, 50 insertions(+), 18 deletions(-) diff --git a/app-mincompile/src/main/java/com/mapbox/navigation/examples/mincompile/DropInUIActivity.kt b/app-mincompile/src/main/java/com/mapbox/navigation/examples/mincompile/DropInUIActivity.kt index 73125df4e12..d37f93eb0cf 100644 --- a/app-mincompile/src/main/java/com/mapbox/navigation/examples/mincompile/DropInUIActivity.kt +++ b/app-mincompile/src/main/java/com/mapbox/navigation/examples/mincompile/DropInUIActivity.kt @@ -3,8 +3,6 @@ package com.mapbox.navigation.examples.mincompile import androidx.appcompat.app.AppCompatActivity import android.os.Bundle import com.mapbox.navigation.examples.mincompile.databinding.LayoutActivityDropinBinding -import com.mapbox.navigation.examples.mincompile.databinding.LayoutActivityMainBinding -import com.mapbox.navigation.examples.mincompile.databinding.LayoutActivityNavigationBinding class DropInUIActivity : AppCompatActivity() { private lateinit var binding: LayoutActivityDropinBinding diff --git a/libnavigation-base/src/main/java/com/mapbox/navigation/base/internal/route/RouteCompatibilityCache.kt b/libnavigation-base/src/main/java/com/mapbox/navigation/base/internal/route/RouteCompatibilityCache.kt index c15fd9c68d3..d45b222c6e3 100644 --- a/libnavigation-base/src/main/java/com/mapbox/navigation/base/internal/route/RouteCompatibilityCache.kt +++ b/libnavigation-base/src/main/java/com/mapbox/navigation/base/internal/route/RouteCompatibilityCache.kt @@ -1,11 +1,11 @@ package com.mapbox.navigation.base.internal.route -import android.util.LruCache import com.mapbox.api.directions.v5.models.DirectionsRoute import com.mapbox.navigation.base.route.NavigationRoute +import java.util.concurrent.ConcurrentHashMap /** - * **Internal** cache object that aims to improve the performance performance of compatibility functions + * **Internal** cache object that aims to improve the performance of compatibility functions * that transform the [DirectionsRoute] route into a [NavigationRoute]. * * It caches up to 3 randomly created [NavigationRoute] instances, @@ -15,39 +15,50 @@ import com.mapbox.navigation.base.route.NavigationRoute * This should be relatively efficient since we treat `MapboxDirectionsSession` as the source of truth and `MapboxNavigation` will clear it on its `onDestroy` as well. */ object RouteCompatibilityCache { - private val directionsSessionCache = mutableListOf() - private val creationCache = LruCache(3) - private val lock = Any() + private const val MAX_CACHE_SIZE = 3 + private val creationCache = + ConcurrentHashMap(MAX_CACHE_SIZE) /** * Use to put a result of a random route creation to cache. */ fun cacheCreationResult(routes: List) { - synchronized(lock) { - routes.forEach { - creationCache.put(it.directionsRoute, it) - } + routes.forEach { + creationCache[it.directionsRoute] = NavigationRoutePackage(it, System.nanoTime()) } + maintainCacheSize() } /** * Use to put all routes tracked by `MapboxDirectionsSession` to cache (and clear everything else). */ fun setDirectionsSessionResult(routes: List) { - synchronized(lock) { - creationCache.evictAll() - directionsSessionCache.clear() - directionsSessionCache.addAll(routes) + creationCache.clear() + routes.forEach { + creationCache[it.directionsRoute] = NavigationRoutePackage(it, System.nanoTime()) } + maintainCacheSize() } /** * Get a cached [NavigationRoute] if there is one that wraps the provided [DirectionsRoute] (based on equality) or `null`. */ fun getFor(directionsRoute: DirectionsRoute): NavigationRoute? { - synchronized(lock) { - return directionsSessionCache.find { it.directionsRoute == directionsRoute } - ?: creationCache.get(directionsRoute) + return creationCache[directionsRoute]?.route + } + + /** + * Sorts the items in the cache by timestamp and removes the oldest items in order to maintain + * a size no greater than MAX_CACHE_SIZE. + */ + private fun maintainCacheSize() { + if (creationCache.size > MAX_CACHE_SIZE) { + val items = creationCache.map { + Pair(it.key, it.value) + }.sortedBy { it.second.timestamp } + val overSize = items.size - MAX_CACHE_SIZE + items.take(overSize).forEach { creationCache.remove(it.first) } } } + private data class NavigationRoutePackage(val route: NavigationRoute, val timestamp: Long) } diff --git a/libnavigation-base/src/test/java/com/mapbox/navigation/base/internal/route/RouteCompatibilityCacheTest.kt b/libnavigation-base/src/test/java/com/mapbox/navigation/base/internal/route/RouteCompatibilityCacheTest.kt index 90decc10ba4..458049c425b 100644 --- a/libnavigation-base/src/test/java/com/mapbox/navigation/base/internal/route/RouteCompatibilityCacheTest.kt +++ b/libnavigation-base/src/test/java/com/mapbox/navigation/base/internal/route/RouteCompatibilityCacheTest.kt @@ -98,4 +98,27 @@ class RouteCompatibilityCacheTest { assertEquals(navigationRoute1, RouteCompatibilityCache.getFor(directionsRouteMock1)) assertEquals(navigationRoute2, RouteCompatibilityCache.getFor(directionsRouteMock2)) } + + @Test + fun `cache size does not exceed maximum`() { + val route1 = mockk { + every { directionsRoute } returns mockk() + } + val route2 = mockk { + every { directionsRoute } returns mockk() + } + val route3 = mockk { + every { directionsRoute } returns mockk() + } + val route4 = mockk { + every { directionsRoute } returns mockk() + } + + RouteCompatibilityCache.setDirectionsSessionResult(listOf(route1, route2, route3, route4)) + + assertNull(RouteCompatibilityCache.getFor(route1.directionsRoute)) + assertEquals(route2, RouteCompatibilityCache.getFor(route2.directionsRoute)) + assertEquals(route3, RouteCompatibilityCache.getFor(route3.directionsRoute)) + assertEquals(route4, RouteCompatibilityCache.getFor(route4.directionsRoute)) + } }