From f63d184f9c4147f9ff68142d67de884817bf9d97 Mon Sep 17 00:00:00 2001 From: bidetofevil Date: Fri, 20 Sep 2024 14:52:20 -0700 Subject: [PATCH 1/2] Optimize background activity caching --- .../PeriodicBackgroundActivityCacher.kt | 16 ++- .../PeriodicBackgroundActivityCacherTest.kt | 123 ++++++++++++++++++ 2 files changed, 132 insertions(+), 7 deletions(-) create mode 100644 embrace-android-core/src/test/java/io/embrace/android/embracesdk/internal/session/caching/PeriodicBackgroundActivityCacherTest.kt diff --git a/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/session/caching/PeriodicBackgroundActivityCacher.kt b/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/session/caching/PeriodicBackgroundActivityCacher.kt index 247ebbc49b..f3ad9aa347 100644 --- a/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/session/caching/PeriodicBackgroundActivityCacher.kt +++ b/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/session/caching/PeriodicBackgroundActivityCacher.kt @@ -18,11 +18,10 @@ class PeriodicBackgroundActivityCacher( ) { private companion object { - /** * Minimum time between writes of the background activity to disk */ - private const val MIN_INTERVAL_BETWEEN_SAVES: Long = 5000 + private const val MIN_INTERVAL_BETWEEN_SAVES: Long = 2000L } private var lastSaved: Long = 0 @@ -44,11 +43,14 @@ class PeriodicBackgroundActivityCacher( logger.trackInternalError(InternalErrorType.BG_SESSION_CACHE_FAIL, ex) } } - scheduledFuture = worker.schedule( - action, - delay, - TimeUnit.MILLISECONDS - ) + + if (scheduledFuture?.isDone != false) { + scheduledFuture = worker.schedule( + action, + delay, + TimeUnit.MILLISECONDS + ) + } } private fun calculateDelay(): Long { diff --git a/embrace-android-core/src/test/java/io/embrace/android/embracesdk/internal/session/caching/PeriodicBackgroundActivityCacherTest.kt b/embrace-android-core/src/test/java/io/embrace/android/embracesdk/internal/session/caching/PeriodicBackgroundActivityCacherTest.kt new file mode 100644 index 0000000000..960daaee65 --- /dev/null +++ b/embrace-android-core/src/test/java/io/embrace/android/embracesdk/internal/session/caching/PeriodicBackgroundActivityCacherTest.kt @@ -0,0 +1,123 @@ +package io.embrace.android.embracesdk.internal.session.caching + +import io.embrace.android.embracesdk.concurrency.BlockingScheduledExecutorService +import io.embrace.android.embracesdk.fakes.FakeClock +import io.embrace.android.embracesdk.fakes.FakeEmbLogger +import io.embrace.android.embracesdk.fakes.fakeSessionEnvelope +import io.embrace.android.embracesdk.internal.logging.EmbLogger +import io.embrace.android.embracesdk.internal.worker.BackgroundWorker +import org.junit.Assert.assertEquals +import org.junit.Before +import org.junit.Test +import java.util.concurrent.CountDownLatch +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicInteger + +internal class PeriodicBackgroundActivityCacherTest { + + private val clock: FakeClock = FakeClock() + private val logger: EmbLogger = FakeEmbLogger() + private lateinit var executor: BlockingScheduledExecutorService + private lateinit var worker: BackgroundWorker + private lateinit var cacher: PeriodicBackgroundActivityCacher + private lateinit var executionCount: AtomicInteger + + @Before + fun setUp() { + executor = BlockingScheduledExecutorService( + fakeClock = clock, + blockingMode = true + ) + worker = BackgroundWorker(executor) + cacher = PeriodicBackgroundActivityCacher( + clock = clock, + logger = logger, + worker = worker + ) + executionCount = AtomicInteger(0) + } + + @Test + fun `do not schedule save if there is a pending one`() { + queueScheduleSave() + queueScheduleSave() + val latch = queueCompletionTask() + executor.blockingMode = false + latch.assertCountedDown() + assertEquals(1, executionCount.get()) + } + + @Test + fun `schedule save if previous one has already run`() { + val latch1 = queueScheduleSave() + executor.runCurrentlyBlocked() + latch1.assertCountedDown() + assertEquals(0, executor.scheduledTasksCount()) + val latch2 = queueScheduleSave() + executor.moveForwardAndRunBlocked(10_000L) + latch2.assertCountedDown() + assertEquals(2, executionCount.get()) + } + + @Test + fun `scheduled save will only run after minimum delay has elapsed`() { + val latch1 = queueScheduleSave() + executor.runCurrentlyBlocked() + latch1.assertCountedDown() + assertEquals(0, executor.scheduledTasksCount()) + val latch2 = queueScheduleSave() + assertEquals(1, executor.scheduledTasksCount()) + executor.moveForwardAndRunBlocked(1999) + executor.blockingMode = false + queueCompletionTask().assertCountedDown() + assertEquals(1, latch2.count) + assertEquals(1, executionCount.get()) + executor.moveForwardAndRunBlocked(2) + latch2.assertCountedDown() + assertEquals(2, executionCount.get()) + } + + @Test + fun `stopping cacher prevents execution`() { + queueScheduleSave() + cacher.stop() + val latch = queueCompletionTask() + executor.blockingMode = false + latch.assertCountedDown() + assertEquals(0, executionCount.get()) + } + + @Test + fun `stopping allows another save to be scheduled`() { + queueScheduleSave() + cacher.stop() + queueScheduleSave() + val latch = queueCompletionTask() + executor.blockingMode = false + latch.assertCountedDown() + assertEquals(1, executionCount.get()) + } + + private fun queueScheduleSave(): CountDownLatch { + val latch = CountDownLatch(1) + cacher.scheduleSave { + executionCount.incrementAndGet() + latch.countDown() + fakeSessionEnvelope() + } + return latch + } + + private fun queueCompletionTask(): CountDownLatch { + val latch = CountDownLatch(1) + executor.submit { + latch.countDown() + } + return latch + } + + private fun CountDownLatch.assertCountedDown() { + await(1, TimeUnit.SECONDS) + assertEquals(0, count) + } +} From 993afa1213e8bf1086dc50fbec7abee9d4da8241 Mon Sep 17 00:00:00 2001 From: bidetofevil Date: Fri, 20 Sep 2024 16:04:17 -0700 Subject: [PATCH 2/2] Revert logic change and keep only the decrease in the delay --- .../PeriodicBackgroundActivityCacher.kt | 13 ++++----- .../PeriodicBackgroundActivityCacherTest.kt | 29 ++----------------- 2 files changed, 8 insertions(+), 34 deletions(-) diff --git a/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/session/caching/PeriodicBackgroundActivityCacher.kt b/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/session/caching/PeriodicBackgroundActivityCacher.kt index f3ad9aa347..cffd102d4d 100644 --- a/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/session/caching/PeriodicBackgroundActivityCacher.kt +++ b/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/session/caching/PeriodicBackgroundActivityCacher.kt @@ -43,14 +43,11 @@ class PeriodicBackgroundActivityCacher( logger.trackInternalError(InternalErrorType.BG_SESSION_CACHE_FAIL, ex) } } - - if (scheduledFuture?.isDone != false) { - scheduledFuture = worker.schedule( - action, - delay, - TimeUnit.MILLISECONDS - ) - } + scheduledFuture = worker.schedule( + action, + delay, + TimeUnit.MILLISECONDS + ) } private fun calculateDelay(): Long { diff --git a/embrace-android-core/src/test/java/io/embrace/android/embracesdk/internal/session/caching/PeriodicBackgroundActivityCacherTest.kt b/embrace-android-core/src/test/java/io/embrace/android/embracesdk/internal/session/caching/PeriodicBackgroundActivityCacherTest.kt index 960daaee65..98779f5d3c 100644 --- a/embrace-android-core/src/test/java/io/embrace/android/embracesdk/internal/session/caching/PeriodicBackgroundActivityCacherTest.kt +++ b/embrace-android-core/src/test/java/io/embrace/android/embracesdk/internal/session/caching/PeriodicBackgroundActivityCacherTest.kt @@ -38,7 +38,7 @@ internal class PeriodicBackgroundActivityCacherTest { } @Test - fun `do not schedule save if there is a pending one`() { + fun `do not save more than once if delay time has not elapsed`() { queueScheduleSave() queueScheduleSave() val latch = queueCompletionTask() @@ -48,19 +48,7 @@ internal class PeriodicBackgroundActivityCacherTest { } @Test - fun `schedule save if previous one has already run`() { - val latch1 = queueScheduleSave() - executor.runCurrentlyBlocked() - latch1.assertCountedDown() - assertEquals(0, executor.scheduledTasksCount()) - val latch2 = queueScheduleSave() - executor.moveForwardAndRunBlocked(10_000L) - latch2.assertCountedDown() - assertEquals(2, executionCount.get()) - } - - @Test - fun `scheduled save will only run after minimum delay has elapsed`() { + fun `scheduled save will run once minimum delay time has elapsed`() { val latch1 = queueScheduleSave() executor.runCurrentlyBlocked() latch1.assertCountedDown() @@ -78,7 +66,7 @@ internal class PeriodicBackgroundActivityCacherTest { } @Test - fun `stopping cacher prevents execution`() { + fun `stopping cacher prevents execution of the pending scheduled save`() { queueScheduleSave() cacher.stop() val latch = queueCompletionTask() @@ -87,17 +75,6 @@ internal class PeriodicBackgroundActivityCacherTest { assertEquals(0, executionCount.get()) } - @Test - fun `stopping allows another save to be scheduled`() { - queueScheduleSave() - cacher.stop() - queueScheduleSave() - val latch = queueCompletionTask() - executor.blockingMode = false - latch.assertCountedDown() - assertEquals(1, executionCount.get()) - } - private fun queueScheduleSave(): CountDownLatch { val latch = CountDownLatch(1) cacher.scheduleSave {