Cache decrypted getAll results in CachingApplicationDao

heapprofd profiling showed NotificationSubscription.updateFilter()'s ~30s relay-refresh cycle re-running ApplicationDao.getAll on the raw Room dao, re-decrypting every application row through AndroidKeyStore (a keystore2 binder round-trip per encrypted field) forever. That path was the app's dominant native allocator: ~15% of live native memory plus all periodic ~400KB allocation bursts (1.33MB/90s steady-state churn). getAll is now a read-through cache: a second small LruCache keyed by account pubKey, handing out defensive copies. Every application-table mutation evicts the affected account's entry (or all entries when only the app key is known); permission-only writes never do. Call sites that could bypass the wrapper are routed through the cached dao so the cache cannot go stale: updateFilter's read, HistoryDao's updateLastUsed write, and ConnectivityService's reconnect write. Re-profiled post-fix: decrypt-path stacks 0 rows/0 bytes (was 65 rows/ 1.14MB per 90s); steady-state churn ~0.26MB/90s (was ~1.33MB); no periodic bursts.
2026-10-05 10:58:23 +00:00 · 2026-08-21 06:01:02 -03:00
parent ee745e806c
commit 591ed2324d
6 changed files with 268 additions and 10 deletions
@@ -17,6 +17,16 @@ import androidx.paging.PagingSource
 * (coarse-grained per-app). Every mutation in [ApplicationDao] is scoped to a
 * single app key, so this matches the natural granularity without forcing
 * callers to know which exact (type, kind, relay) tuple they touched.
+ *
+ * [getAll] results are cached per account pubKey in a separate small LRU.
+ * That query decrypts every application row (AndroidKeyStore AES-GCM, a binder
+ * hop to keystore2 per field) and [com.greenart7c3.nostrsigner.service.NotificationSubscription]
+ * re-runs it on every ~30s relay-refresh cycle, which made it the app's
+ * dominant native allocator. Any mutation of the `application` table evicts
+ * the affected account's entry (or all entries when only the app key — not
+ * the owning account — is known). Cached lists are handed out as defensive
+ * copies; callers must not expect in-place edits of returned entities to be
+ * visible (all current call sites are read-only consumers).
 */
 class CachingApplicationDao(
    private val delegate: ApplicationDao,
@@ -38,12 +48,22 @@ class CachingApplicationDao(

    private val cache = LruCache<Key, Value>(MAX_ENTRIES)

+    // One entry per account (key = account pubKey); a handful at most.
+    private val getAllCache = LruCache<String, List<ApplicationEntity>>(GET_ALL_MAX_ENTRIES)
+
    private fun lookup(key: Key): Value? = synchronized(cache) { cache.get(key) }

    private fun store(key: Key, value: Value) {
        synchronized(cache) { cache.put(key, value) }
    }

+    /** Evicts the cached [getAll] list for [pubKey], or every list when null. */
+    private fun invalidateGetAll(pubKey: String?) {
+        synchronized(getAllCache) {
+            if (pubKey != null) getAllCache.remove(pubKey) else getAllCache.evictAll()
+        }
+    }
+
    private fun invalidateApp(pkKey: String) {
        synchronized(cache) {
            val toRemove = cache.snapshot().keys.filter { it.pkKey == pkKey }
@@ -122,6 +142,7 @@ class CachingApplicationDao(
    override suspend fun insertApplicationWithPermissions(application: ApplicationWithPermissions) {
        delegate.insertApplicationWithPermissions(application)
        invalidateApp(application.application.key)
+        invalidateGetAll(application.application.pubKey)
    }

    override suspend fun deletePermissions(key: String) {
@@ -157,11 +178,15 @@ class CachingApplicationDao(
    override suspend fun delete(entity: ApplicationEntity) {
        delegate.delete(entity)
        invalidateApp(entity.key)
+        invalidateGetAll(entity.pubKey)
    }

    override suspend fun delete(key: String) {
        delegate.delete(key)
        invalidateApp(key)
+        // Only the app key is known here, not the owning account, so drop every
+        // getAll entry — there is at most one per account.
+        invalidateGetAll(null)
    }

    // Affects many apps at once (timestamp predicate), so wipe the whole cache.
@@ -172,13 +197,21 @@ class CachingApplicationDao(

    override suspend fun deleteOldApplications(time: Long): Int {
        val n = delegate.deleteOldApplications(time)
-        if (n > 0) invalidateAll()
+        if (n > 0) {
+            invalidateAll()
+            invalidateGetAll(null)
+        }
        return n
    }

    // -------- pass-through (not cached, not invalidating) --------

-    override suspend fun getAll(pubKey: String): List<ApplicationEntity> = delegate.getAll(pubKey)
+    override suspend fun getAll(pubKey: String): List<ApplicationEntity> {
+        synchronized(getAllCache) { getAllCache.get(pubKey) }?.let { return it.toList() }
+        val result = delegate.getAll(pubKey)
+        synchronized(getAllCache) { getAllCache.put(pubKey, result) }
+        return result.toList()
+    }

    override suspend fun getAllNotConnected(): List<ApplicationWithPermissions> = delegate.getAllNotConnected()

@@ -211,23 +244,28 @@ class CachingApplicationDao(
        // ApplicationEntity columns like signPolicy and lastUsed feed cached reads
        // (getSignPolicy), so invalidate the affected app's entries.
        invalidateApp(event.key)
+        invalidateGetAll(event.pubKey)
        return result
    }

    override fun insertAll(events: List<ApplicationEntity>): List<Long>? {
        val result = delegate.insertAll(events)
        events.forEach { invalidateApp(it.key) }
+        events.asSequence().map { it.pubKey }.distinct().forEach(::invalidateGetAll)
        return result
    }

    override suspend fun updateLastUsed(key: String, time: Long) {
        delegate.updateLastUsed(key, time)
-        // lastUsed doesn't affect any cached read, so no invalidation needed.
+        // lastUsed doesn't affect any cached read of the permission cache, but it
+        // is a column of getAll's projection, so the per-account list goes stale.
+        invalidateGetAll(null)
    }

    override suspend fun updateNameAndIcon(key: String, name: String, icon: String) {
        delegate.updateNameAndIcon(key, name, icon)
        invalidateApp(key)
+        invalidateGetAll(null)
    }

    // -------- *Raw delegations (envelope-encrypted persistence boundary) --------
@@ -255,9 +293,14 @@ class CachingApplicationDao(

    override suspend fun getAllWithLocalKeyRaw(pubKey: String): List<ApplicationEntity> = delegate.getAllWithLocalKeyRaw(pubKey)

-    // Key-rotation write path: secret/localKey never feed a cached read, so a
-    // plain delegation with no invalidation is correct.
-    override suspend fun updateEncryptedColumnsRaw(key: String, secret: String, localKey: String) = delegate.updateEncryptedColumnsRaw(key, secret, localKey)
+    // Key-rotation write path: secret/localKey never feed a cached read of the
+    // permission cache, but they ARE columns of getAll's projection, so evict
+    // the per-account lists (SecureCryptoHelper.rotateKey calls this through
+    // the caching wrapper, ciphertext in / ciphertext out).
+    override suspend fun updateEncryptedColumnsRaw(key: String, secret: String, localKey: String) {
+        delegate.updateEncryptedColumnsRaw(key, secret, localKey)
+        invalidateGetAll(null)
+    }

    override suspend fun insertApplicationRaw(event: ApplicationEntity): Long? = delegate.insertApplicationRaw(event)

@@ -271,5 +314,6 @@ class CachingApplicationDao(

    companion object {
        private const val MAX_ENTRIES = 512
+        private const val GET_ALL_MAX_ENTRIES = 8
    }
 }
@@ -129,7 +129,9 @@ interface HistoryDao {
                val lastUsed = entities.maxByOrNull { it.time }?.time ?: TimeUtils.now()
                val pkKey = entities.firstOrNull()?.pkKey
                if (pkKey != null) {
-                    Amber.instance.getDatabase(npub).dao().updateLastUsed(pkKey, lastUsed)
+                    // Cached dao so the write invalidates CachingApplicationDao's
+                    // getAll entry (lastUsed is a column of that projection).
+                    Amber.instance.dao(npub).updateLastUsed(pkKey, lastUsed)
                }
            }
        } catch (e: Exception) {
@@ -113,10 +113,13 @@ class ConnectivityService : Service() {
        LocalPreferences.allSavedAccounts(this).forEach {
            Amber.instance.databases[it.npub] = Amber.instance.getDatabase(it.npub)
            Amber.instance.applicationIOScope.launch {
-                Amber.instance.databases[it.npub]?.dao()?.getAllNotConnected()?.forEach { app ->
+                // Cached dao so the write invalidates CachingApplicationDao's getAll
+                // entry for this account (the service can restart in a live process).
+                val dao = Amber.instance.dao(it.npub)
+                dao.getAllNotConnected()?.forEach { app ->
                    if (app.application.secret.isNotEmpty() && app.application.secret != app.application.key) {
                        app.application.isConnected = true
-                        Amber.instance.databases[it.npub]?.dao()?.insertApplicationWithPermissions(app)
+                        dao.insertApplicationWithPermissions(app)
                    }
                }
            }
@@ -86,7 +86,12 @@ class NotificationSubscription(
        LocalPreferences.allAccounts(appContext).forEach { account ->
            val since = computeSince()

-            val allConnections = Amber.instance.getDatabase(account.npub).dao().getAll(account.hexKey)
+            // Cached dao: getAll() is served from CachingApplicationDao's per-account
+            // LRU. updateFilter re-runs on every ~30s relay refresh, and an uncached
+            // getAll re-decrypts every application row through AndroidKeyStore
+            // (a keystore2 binder round-trip per encrypted field) each cycle —
+            // previously the app's dominant native allocator.
+            val allConnections = Amber.instance.dao(account.npub).getAll(account.hexKey)
            val connectionsWithLocalKey = allConnections.filter { it.localKey.isNotEmpty() }
            val hasLegacyConnections = allConnections.any { it.localKey.isEmpty() && it.relays.isNotEmpty() }

@@ -0,0 +1,200 @@
+package com.greenart7c3.nostrsigner.database
+
+import io.mockk.coEvery
+import io.mockk.coVerify
+import io.mockk.mockk
+import kotlinx.coroutines.runBlocking
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNotSame
+import org.junit.Before
+import org.junit.Test
+
+/**
+ * Verifies the per-account getAll() read-through cache introduced because
+ * NotificationSubscription.updateFilter() re-runs getAll on every ~30s relay
+ * refresh and each call used to re-decrypt every application row through
+ * AndroidKeyStore. The delegate must be hit once per cache lifetime, and every
+ * application-table mutation must evict the cached list.
+ */
+class CachingApplicationDaoTest {
+    private lateinit var delegate: ApplicationDao
+    private lateinit var dao: CachingApplicationDao
+
+    private val accountKey = "accounthexpubkey"
+
+    private fun entity(key: String = "app1") = ApplicationEntity.empty().copy(
+        key = key,
+        pubKey = accountKey,
+        localKey = "ab".repeat(32),
+    )
+
+    @Before
+    fun setUp() {
+        delegate = mockk(relaxed = true)
+        dao = CachingApplicationDao(delegate)
+    }
+
+    private suspend fun stubGetAll(vararg entities: ApplicationEntity) {
+        coEvery { delegate.getAll(accountKey) } returns entities.toList()
+    }
+
+    @Test
+    fun `getAll hits the delegate once per cache lifetime`() = runBlocking {
+        stubGetAll(entity())
+
+        dao.getAll(accountKey)
+        dao.getAll(accountKey)
+        dao.getAll(accountKey)
+
+        coVerify(exactly = 1) { delegate.getAll(accountKey) }
+    }
+
+    @Test
+    fun `getAll returns equal lists on cache hits`() = runBlocking {
+        val first = entity()
+        stubGetAll(first)
+
+        assertEquals(listOf(first), dao.getAll(accountKey))
+        assertEquals(listOf(first), dao.getAll(accountKey))
+    }
+
+    @Test
+    fun `returned list is a defensive copy of the cached list`() = runBlocking {
+        stubGetAll(entity())
+
+        val first = dao.getAll(accountKey)
+        val second = dao.getAll(accountKey)
+
+        // Cache hits must not hand out the same mutable list instance callers
+        // could alias; equality stays intact.
+        assertNotSame(first, second)
+        assertEquals(first, second)
+    }
+
+    @Test
+    fun `insertApplication evicts the owning account's cached list`() = runBlocking {
+        stubGetAll(entity())
+        dao.getAll(accountKey)
+
+        dao.insertApplication(entity())
+
+        dao.getAll(accountKey)
+        coVerify(exactly = 2) { delegate.getAll(accountKey) }
+    }
+
+    @Test
+    fun `insertAll evicts the owning account's cached list`() = runBlocking {
+        stubGetAll(entity())
+        dao.getAll(accountKey)
+
+        dao.insertAll(listOf(entity()))
+
+        dao.getAll(accountKey)
+        coVerify(exactly = 2) { delegate.getAll(accountKey) }
+    }
+
+    @Test
+    fun `insertApplicationWithPermissions evicts the owning account's cached list`() = runBlocking {
+        stubGetAll(entity())
+        dao.getAll(accountKey)
+
+        val withPermissions = ApplicationWithPermissions(entity(), mutableListOf())
+        dao.insertApplicationWithPermissions(withPermissions)
+
+        dao.getAll(accountKey)
+        coVerify(exactly = 2) { delegate.getAll(accountKey) }
+    }
+
+    @Test
+    fun `delete by entity evicts the owning account's cached list`() = runBlocking {
+        stubGetAll(entity())
+        dao.getAll(accountKey)
+
+        dao.delete(entity())
+
+        dao.getAll(accountKey)
+        coVerify(exactly = 2) { delegate.getAll(accountKey) }
+    }
+
+    @Test
+    fun `delete by key evicts all cached lists`() = runBlocking {
+        stubGetAll(entity())
+        dao.getAll(accountKey)
+
+        dao.delete("app1")
+
+        dao.getAll(accountKey)
+        coVerify(exactly = 2) { delegate.getAll(accountKey) }
+    }
+
+    @Test
+    fun `updateLastUsed evicts all cached lists`() = runBlocking {
+        stubGetAll(entity())
+        dao.getAll(accountKey)
+
+        dao.updateLastUsed("app1", 123L)
+
+        dao.getAll(accountKey)
+        coVerify(exactly = 2) { delegate.getAll(accountKey) }
+    }
+
+    @Test
+    fun `updateNameAndIcon evicts all cached lists`() = runBlocking {
+        stubGetAll(entity())
+        dao.getAll(accountKey)
+
+        dao.updateNameAndIcon("app1", "name", "icon")
+
+        dao.getAll(accountKey)
+        coVerify(exactly = 2) { delegate.getAll(accountKey) }
+    }
+
+    @Test
+    fun `updateEncryptedColumnsRaw evicts all cached lists`() = runBlocking {
+        stubGetAll(entity())
+        dao.getAll(accountKey)
+
+        dao.updateEncryptedColumnsRaw("app1", "secret", "localKey")
+
+        dao.getAll(accountKey)
+        coVerify(exactly = 2) { delegate.getAll(accountKey) }
+    }
+
+    @Test
+    fun `deleteOldApplications evicts cached lists when rows are removed`() = runBlocking {
+        stubGetAll(entity())
+        dao.getAll(accountKey)
+
+        coEvery { delegate.deleteOldApplications(any()) } returns 1
+        dao.deleteOldApplications(0L)
+
+        dao.getAll(accountKey)
+        coVerify(exactly = 2) { delegate.getAll(accountKey) }
+    }
+
+    @Test
+    fun `permission-only writes do not evict the cached list`() = runBlocking {
+        stubGetAll(entity())
+        dao.getAll(accountKey)
+
+        dao.deletePermissions("app1")
+
+        dao.getAll(accountKey)
+        coVerify(exactly = 1) { delegate.getAll(accountKey) }
+    }
+
+    @Test
+    fun `accounts are cached independently`() = runBlocking {
+        val otherAccount = "otheraccounthexkey"
+        coEvery { delegate.getAll(accountKey) } returns listOf(entity())
+        coEvery { delegate.getAll(otherAccount) } returns emptyList()
+
+        dao.getAll(accountKey)
+        dao.getAll(otherAccount)
+        dao.getAll(accountKey)
+        dao.getAll(otherAccount)
+
+        coVerify(exactly = 1) { delegate.getAll(accountKey) }
+        coVerify(exactly = 1) { delegate.getAll(otherAccount) }
+    }
+}
@@ -7,6 +7,7 @@ import com.greenart7c3.nostrsigner.LocalPreferences
 import com.greenart7c3.nostrsigner.database.AppDatabase
 import com.greenart7c3.nostrsigner.database.ApplicationDao
 import com.greenart7c3.nostrsigner.database.ApplicationEntity
+import com.greenart7c3.nostrsigner.database.CachingApplicationDao
 import com.greenart7c3.nostrsigner.models.Account
 import com.vitorpamplona.quartz.nip01Core.core.Event
 import com.vitorpamplona.quartz.nip01Core.relay.client.NostrClient
@@ -51,6 +52,9 @@ class NotificationSubscriptionTest {

        val amber = mockk<Amber>(relaxed = true)
        every { amber.getDatabase(any()) } returns database
+        // updateFilter reads through the caching wrapper; a fresh wrapper per call
+        // mirrors per-test dao state without cross-test cache leakage.
+        every { amber.dao(any()) } answers { CachingApplicationDao(dao) }
        every { amber.notificationCache } returns LruCache(512)
        installAmberInstance(amber)