From 5a908c1d01570e46bb48d9b8b4764ce3cdffc2bd Mon Sep 17 00:00:00 2001 From: David Perez Date: Fri, 24 May 2024 11:07:31 -0500 Subject: [PATCH] Add manager that hints to the garbage collector to collect the garbage (#1387) --- .../manager/di/PlatformManagerModule.kt | 10 ++ .../garbage/GarbageCollectionManager.kt | 12 ++ .../garbage/GarbageCollectionManagerImpl.kt | 46 +++++++ .../platform/base/FakeDispatcherManager.kt | 12 +- .../garbage/GarbageCollectionManagerTest.kt | 113 ++++++++++++++++++ .../x8bit/bitwarden/data/util/TestHelpers.kt | 13 ++ 6 files changed, 199 insertions(+), 7 deletions(-) create mode 100644 app/src/main/java/com/x8bit/bitwarden/data/platform/manager/garbage/GarbageCollectionManager.kt create mode 100644 app/src/main/java/com/x8bit/bitwarden/data/platform/manager/garbage/GarbageCollectionManagerImpl.kt create mode 100644 app/src/test/java/com/x8bit/bitwarden/data/platform/manager/garbage/GarbageCollectionManagerTest.kt diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/di/PlatformManagerModule.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/di/PlatformManagerModule.kt index bc6e34d20..02fa8db46 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/di/PlatformManagerModule.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/di/PlatformManagerModule.kt @@ -33,6 +33,8 @@ import com.x8bit.bitwarden.data.platform.manager.clipboard.BitwardenClipboardMan import com.x8bit.bitwarden.data.platform.manager.clipboard.BitwardenClipboardManagerImpl import com.x8bit.bitwarden.data.platform.manager.dispatcher.DispatcherManager import com.x8bit.bitwarden.data.platform.manager.dispatcher.DispatcherManagerImpl +import com.x8bit.bitwarden.data.platform.manager.garbage.GarbageCollectionManager +import com.x8bit.bitwarden.data.platform.manager.garbage.GarbageCollectionManagerImpl import com.x8bit.bitwarden.data.platform.repository.EnvironmentRepository import com.x8bit.bitwarden.data.platform.repository.SettingsRepository import com.x8bit.bitwarden.data.vault.datasource.sdk.BitwardenFeatureFlagManager @@ -97,6 +99,14 @@ object PlatformManagerModule { @Singleton fun provideBitwardenDispatchers(): DispatcherManager = DispatcherManagerImpl() + @Provides + @Singleton + fun provideGarbageCollectionManager( + dispatcherManager: DispatcherManager, + ): GarbageCollectionManager = GarbageCollectionManagerImpl( + dispatcherManager = dispatcherManager, + ) + @Provides @Singleton fun provideSdkClientManager( diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/garbage/GarbageCollectionManager.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/garbage/GarbageCollectionManager.kt new file mode 100644 index 000000000..cd09dcb89 --- /dev/null +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/garbage/GarbageCollectionManager.kt @@ -0,0 +1,12 @@ +package com.x8bit.bitwarden.data.platform.manager.garbage + +/** + * A manager for interfacing with the garbage collector. + */ +interface GarbageCollectionManager { + /** + * Calls the garbage collector on the [Runtime] in an effort to clear the unused resources in + * the heap. + */ + fun tryCollect() +} diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/garbage/GarbageCollectionManagerImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/garbage/GarbageCollectionManagerImpl.kt new file mode 100644 index 000000000..e048107bd --- /dev/null +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/garbage/GarbageCollectionManagerImpl.kt @@ -0,0 +1,46 @@ +package com.x8bit.bitwarden.data.platform.manager.garbage + +import com.x8bit.bitwarden.data.platform.manager.dispatcher.DispatcherManager +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Job +import kotlinx.coroutines.delay +import kotlinx.coroutines.launch + +/** + * Default implementation of the [GarbageCollectionManager]. + */ +@Suppress("ExplicitGarbageCollectionCall") +class GarbageCollectionManagerImpl( + private val garbageCollector: () -> Unit = { Runtime.getRuntime().gc() }, + dispatcherManager: DispatcherManager, +) : GarbageCollectionManager { + private val unconfinedScope = CoroutineScope(dispatcherManager.unconfined) + private var collectionJob: Job = Job().apply { complete() } + + override fun tryCollect() { + collectionJob.cancel() + collectionJob = unconfinedScope.launch { + delay(timeMillis = GARBAGE_COLLECTION_INITIAL_DELAY_MS) + repeat(times = GARBAGE_COLLECTION_ATTEMPTS) { + delay(timeMillis = GARBAGE_COLLECTION_BASE_BACKOFF_MS * it) + garbageCollector() + } + } + } +} + +/** + * The number of time the garbage collector should be called. + */ +private const val GARBAGE_COLLECTION_ATTEMPTS: Int = 10 + +/** + * The base delay, in milliseconds, between a garbage collection attempt. The duration will be + * multiplied by the number of attempts made thus far. + */ +private const val GARBAGE_COLLECTION_BASE_BACKOFF_MS: Long = 100L + +/** + * The initial delay, in milliseconds, before the first garbage collection attempt. + */ +private const val GARBAGE_COLLECTION_INITIAL_DELAY_MS: Long = 100L diff --git a/app/src/test/java/com/x8bit/bitwarden/data/platform/base/FakeDispatcherManager.kt b/app/src/test/java/com/x8bit/bitwarden/data/platform/base/FakeDispatcherManager.kt index 8eed59a4e..d17bb9a44 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/platform/base/FakeDispatcherManager.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/platform/base/FakeDispatcherManager.kt @@ -13,15 +13,13 @@ import kotlinx.coroutines.test.setMain * A faked implementation of [DispatcherManager] that uses [UnconfinedTestDispatcher]. */ @OptIn(ExperimentalCoroutinesApi::class) -class FakeDispatcherManager : DispatcherManager { - override val default: CoroutineDispatcher = UnconfinedTestDispatcher() - +class FakeDispatcherManager( + override val default: CoroutineDispatcher = UnconfinedTestDispatcher(), + override val io: CoroutineDispatcher = UnconfinedTestDispatcher(), + override val unconfined: CoroutineDispatcher = UnconfinedTestDispatcher(), +) : DispatcherManager { override val main: MainCoroutineDispatcher = Dispatchers.Main - override val io: CoroutineDispatcher = UnconfinedTestDispatcher() - - override val unconfined: CoroutineDispatcher = UnconfinedTestDispatcher() - /** * Updates the main dispatcher to use the provided [dispatcher]. Used in conjunction with * [resetMain]. diff --git a/app/src/test/java/com/x8bit/bitwarden/data/platform/manager/garbage/GarbageCollectionManagerTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/platform/manager/garbage/GarbageCollectionManagerTest.kt new file mode 100644 index 000000000..d25761365 --- /dev/null +++ b/app/src/test/java/com/x8bit/bitwarden/data/platform/manager/garbage/GarbageCollectionManagerTest.kt @@ -0,0 +1,113 @@ +package com.x8bit.bitwarden.data.platform.manager.garbage + +import com.x8bit.bitwarden.data.platform.base.FakeDispatcherManager +import com.x8bit.bitwarden.data.util.advanceTimeByAndRunCurrent +import kotlinx.coroutines.test.StandardTestDispatcher +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Test + +class GarbageCollectionManagerTest { + + private var garbageCollectionCount = 0 + private val dispatcher = StandardTestDispatcher() + + private val garbageCollectionManager: GarbageCollectionManager = GarbageCollectionManagerImpl( + garbageCollector = { garbageCollectionCount++ }, + dispatcherManager = FakeDispatcherManager(unconfined = dispatcher), + ) + + @Test + fun `tryCollect should attempt to garbage collect 10 times in increasing intervals`() { + garbageCollectionManager.tryCollect() + + // We do nothing right away + dispatcher.scheduler.runCurrent() + assertEquals(0, garbageCollectionCount) + + dispatcher.advanceTimeByAndRunCurrent(delayTimeMillis = 100L) + assertEquals(1, garbageCollectionCount) + + dispatcher.advanceTimeByAndRunCurrent(delayTimeMillis = 200L) + assertEquals(2, garbageCollectionCount) + + dispatcher.advanceTimeByAndRunCurrent(delayTimeMillis = 300L) + assertEquals(3, garbageCollectionCount) + + dispatcher.advanceTimeByAndRunCurrent(delayTimeMillis = 400L) + assertEquals(4, garbageCollectionCount) + + dispatcher.advanceTimeByAndRunCurrent(delayTimeMillis = 500L) + assertEquals(5, garbageCollectionCount) + + dispatcher.advanceTimeByAndRunCurrent(delayTimeMillis = 600L) + assertEquals(6, garbageCollectionCount) + + dispatcher.advanceTimeByAndRunCurrent(delayTimeMillis = 700L) + assertEquals(7, garbageCollectionCount) + + dispatcher.advanceTimeByAndRunCurrent(delayTimeMillis = 800L) + assertEquals(8, garbageCollectionCount) + + dispatcher.advanceTimeByAndRunCurrent(delayTimeMillis = 900L) + assertEquals(9, garbageCollectionCount) + + dispatcher.advanceTimeByAndRunCurrent(delayTimeMillis = 1_000L) + assertEquals(10, garbageCollectionCount) + + // We should stop at this point, even 10 seconds later we should not have run again + dispatcher.advanceTimeByAndRunCurrent(delayTimeMillis = 10_000L) + assertEquals(10, garbageCollectionCount) + } + + @Test + fun `tryCollect should restart the intervals when called multiple times`() { + garbageCollectionManager.tryCollect() + + // We do nothing right away + dispatcher.scheduler.runCurrent() + assertEquals(0, garbageCollectionCount) + + dispatcher.advanceTimeByAndRunCurrent(delayTimeMillis = 100L) + assertEquals(1, garbageCollectionCount) + + garbageCollectionManager.tryCollect() + + // We do nothing right away + dispatcher.scheduler.runCurrent() + assertEquals(1, garbageCollectionCount) + + dispatcher.advanceTimeByAndRunCurrent(delayTimeMillis = 100L) + assertEquals(2, garbageCollectionCount) + + dispatcher.advanceTimeByAndRunCurrent(delayTimeMillis = 200L) + assertEquals(3, garbageCollectionCount) + + dispatcher.advanceTimeByAndRunCurrent(delayTimeMillis = 300L) + assertEquals(4, garbageCollectionCount) + + dispatcher.advanceTimeByAndRunCurrent(delayTimeMillis = 400L) + assertEquals(5, garbageCollectionCount) + + dispatcher.advanceTimeByAndRunCurrent(delayTimeMillis = 500L) + assertEquals(6, garbageCollectionCount) + + dispatcher.advanceTimeByAndRunCurrent(delayTimeMillis = 600L) + assertEquals(7, garbageCollectionCount) + + dispatcher.advanceTimeByAndRunCurrent(delayTimeMillis = 700L) + assertEquals(8, garbageCollectionCount) + + dispatcher.advanceTimeByAndRunCurrent(delayTimeMillis = 800L) + assertEquals(9, garbageCollectionCount) + + dispatcher.advanceTimeByAndRunCurrent(delayTimeMillis = 900L) + assertEquals(10, garbageCollectionCount) + + dispatcher.advanceTimeByAndRunCurrent(delayTimeMillis = 1_000L) + assertEquals(11, garbageCollectionCount) + + // We should stop at this point, even 10 seconds later we should not have run again + dispatcher.advanceTimeByAndRunCurrent(delayTimeMillis = 10_000L) + assertEquals(11, garbageCollectionCount) + } +} diff --git a/app/src/test/java/com/x8bit/bitwarden/data/util/TestHelpers.kt b/app/src/test/java/com/x8bit/bitwarden/data/util/TestHelpers.kt index 40de9de26..0b960283e 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/util/TestHelpers.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/util/TestHelpers.kt @@ -3,6 +3,9 @@ package com.x8bit.bitwarden.data.util import com.x8bit.bitwarden.data.platform.datasource.network.di.PlatformNetworkModule import io.mockk.MockKMatcherScope import io.mockk.every +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.TestCoroutineScheduler +import kotlinx.coroutines.test.TestDispatcher import kotlinx.serialization.json.Json import org.junit.jupiter.api.Assertions.assertEquals @@ -47,3 +50,13 @@ inline fun mockBuilder(crossinline block: MockKMatcherScope.(T this.self as T } } + +/** + * A helper method that calls both [TestCoroutineScheduler.advanceTimeBy] and + * [TestCoroutineScheduler.runCurrent]. + */ +@OptIn(ExperimentalCoroutinesApi::class) +fun TestDispatcher.advanceTimeByAndRunCurrent(delayTimeMillis: Long) { + scheduler.advanceTimeBy(delayTimeMillis = delayTimeMillis) + scheduler.runCurrent() +}