Merge pull request #1050 from vector-im/feature/fix_some_perf

Feature/fix some perf
2020-02-26 12:02:36 +01:00 · 2020-02-26 12:02:36 +01:00 · cb73955946
parent 40b4db4a64 bfe15555a1
commit cb73955946
8 changed files with 55 additions and 58 deletions
--- a/CHANGES.md
+++ b/CHANGES.md
@ -11,6 +11,7 @@ Features ✨:
 Improvements 🙌:
 - Migrate to binary QR code verification (#994)
 - Share action is added to room profile and room member profile (#858)
+ - Fix some performance issues with crypto

 Bugfix 🐛:
 - Account creation: wrongly hints that an email can be used to create an account (#941)
--- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/crosssigning/ComputeTrustTask.kt
+++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/crosssigning/ComputeTrustTask.kt
@ -20,6 +20,8 @@ import im.vector.matrix.android.api.extensions.orFalse
 import im.vector.matrix.android.api.session.crypto.crosssigning.MXCrossSigningInfo
 import im.vector.matrix.android.internal.crypto.store.IMXCryptoStore
 import im.vector.matrix.android.internal.task.Task
+import im.vector.matrix.android.internal.util.MatrixCoroutineDispatchers
+import kotlinx.coroutines.withContext
 import javax.inject.Inject

 internal interface ComputeTrustTask : Task<ComputeTrustTask.Params, RoomEncryptionTrustLevel> {
@ -29,14 +31,15 @@ internal interface ComputeTrustTask : Task<ComputeTrustTask.Params, RoomEncrypti
 }

 internal class DefaultComputeTrustTask @Inject constructor(
-        val cryptoStore: IMXCryptoStore
+        private val cryptoStore: IMXCryptoStore,
+        private val coroutineDispatchers: MatrixCoroutineDispatchers
 ) : ComputeTrustTask {

-    override suspend fun execute(params: ComputeTrustTask.Params): RoomEncryptionTrustLevel {
+    override suspend fun execute(params: ComputeTrustTask.Params): RoomEncryptionTrustLevel = withContext(coroutineDispatchers.crypto) {
        val allTrustedUserIds = params.userIds
                .filter { userId -> getUserCrossSigningKeys(userId)?.isTrusted() == true }

-        return if (allTrustedUserIds.isEmpty()) {
+        if (allTrustedUserIds.isEmpty()) {
            RoomEncryptionTrustLevel.Default
        } else {
            // If one of the verified user as an untrusted device -> warning
--- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/database/mapper/RoomSummaryMapper.kt
+++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/database/mapper/RoomSummaryMapper.kt
@ -16,19 +16,12 @@

 package im.vector.matrix.android.internal.database.mapper

-import im.vector.matrix.android.api.session.crypto.CryptoService
 import im.vector.matrix.android.api.session.room.model.RoomSummary
 import im.vector.matrix.android.api.session.room.model.tag.RoomTag
-import im.vector.matrix.android.internal.crypto.algorithms.olm.OlmDecryptionResult
 import im.vector.matrix.android.internal.database.model.RoomSummaryEntity
-import timber.log.Timber
-import java.util.UUID
 import javax.inject.Inject

-internal class RoomSummaryMapper @Inject constructor(
-        private val cryptoService: CryptoService,
-        private val timelineEventMapper: TimelineEventMapper
-) {
+internal class RoomSummaryMapper @Inject constructor(private val timelineEventMapper: TimelineEventMapper) {

    fun map(roomSummaryEntity: RoomSummaryEntity): RoomSummary {
        val tags = roomSummaryEntity.tags.map {
@ -38,22 +31,6 @@ internal class RoomSummaryMapper @Inject constructor(
        val latestEvent = roomSummaryEntity.latestPreviewableEvent?.let {
            timelineEventMapper.map(it, buildReadReceipts = false)
        }
-        if (latestEvent?.root?.isEncrypted() == true && latestEvent.root.mxDecryptionResult == null) {
-            // TODO use a global event decryptor? attache to session and that listen to new sessionId?
-            // for now decrypt sync
-            try {
-                val result = cryptoService.decryptEvent(latestEvent.root, latestEvent.root.roomId + UUID.randomUUID().toString())
-                latestEvent.root.mxDecryptionResult = OlmDecryptionResult(
-                        payload = result.clearEvent,
-                        senderKey = result.senderCurve25519Key,
-                        keysClaimed = result.claimedEd25519Key?.let { mapOf("ed25519" to it) },
-                        forwardingCurve25519KeyChain = result.forwardingCurve25519KeyChain
-                )
-            } catch (e: Throwable) {
-                Timber.d(e)
-            }
-        }
-
        return RoomSummary(
                roomId = roomSummaryEntity.roomId,
                displayName = roomSummaryEntity.displayName ?: "",
--- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/DefaultSession.kt
+++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/DefaultSession.kt
@ -49,6 +49,7 @@ import im.vector.matrix.android.internal.crypto.crosssigning.ShieldTrustUpdater
 import im.vector.matrix.android.internal.database.LiveEntityObserver
 import im.vector.matrix.android.internal.di.SessionId
 import im.vector.matrix.android.internal.di.WorkManagerProvider
+import im.vector.matrix.android.internal.session.room.timeline.TimelineEventDecryptor
 import im.vector.matrix.android.internal.session.sync.SyncTokenStore
 import im.vector.matrix.android.internal.session.sync.job.SyncThread
 import im.vector.matrix.android.internal.session.sync.job.SyncWorker
@ -93,6 +94,7 @@ internal class DefaultSession @Inject constructor(
        private val homeServerCapabilitiesService: Lazy<HomeServerCapabilitiesService>,
        private val accountDataService: Lazy<AccountDataService>,
        private val _sharedSecretStorageService: Lazy<SharedSecretStorageService>,
+        private val timelineEventDecryptor: TimelineEventDecryptor,
        private val shieldTrustUpdater: ShieldTrustUpdater)
    : Session,
        RoomService by roomService.get(),
@ -126,6 +128,7 @@ internal class DefaultSession @Inject constructor(
        isOpen = true
        liveEntityObservers.forEach { it.start() }
        eventBus.register(this)
+        timelineEventDecryptor.start()
        shieldTrustUpdater.start()
    }

@ -163,6 +166,7 @@ internal class DefaultSession @Inject constructor(
    override fun close() {
        assert(isOpen)
        stopSync()
+        timelineEventDecryptor.destroy()
        liveEntityObservers.forEach { it.dispose() }
        cryptoService.get().close()
        isOpen = false
--- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/RoomSummaryUpdater.kt
+++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/RoomSummaryUpdater.kt
@ -17,6 +17,7 @@
 package im.vector.matrix.android.internal.session.room

 import com.zhuinden.monarchy.Monarchy
+import dagger.Lazy
 import im.vector.matrix.android.api.crypto.RoomEncryptionTrustLevel
 import im.vector.matrix.android.api.session.events.model.EventType
 import im.vector.matrix.android.api.session.events.model.toModel
@ -41,17 +42,20 @@ import im.vector.matrix.android.internal.database.query.whereType
 import im.vector.matrix.android.internal.di.UserId
 import im.vector.matrix.android.internal.session.room.membership.RoomDisplayNameResolver
 import im.vector.matrix.android.internal.session.room.membership.RoomMemberHelper
+import im.vector.matrix.android.internal.session.room.timeline.TimelineEventDecryptor
 import im.vector.matrix.android.internal.session.sync.RoomSyncHandler
 import im.vector.matrix.android.internal.session.sync.model.RoomSyncSummary
 import im.vector.matrix.android.internal.session.sync.model.RoomSyncUnreadNotifications
 import io.realm.Realm
 import org.greenrobot.eventbus.EventBus
+import timber.log.Timber
 import javax.inject.Inject

 internal class RoomSummaryUpdater @Inject constructor(
        @UserId private val userId: String,
        private val roomDisplayNameResolver: RoomDisplayNameResolver,
        private val roomAvatarResolver: RoomAvatarResolver,
+        private val timelineEventDecryptor: Lazy<TimelineEventDecryptor>,
        private val eventBus: EventBus,
        private val monarchy: Monarchy) {

@ -141,6 +145,11 @@ internal class RoomSummaryUpdater @Inject constructor(
            roomSummaryEntity.inviterId = null
        }

+        if (latestPreviewableEvent?.root?.type == EventType.ENCRYPTED && latestPreviewableEvent.root?.decryptionResultJson == null) {
+            Timber.v("Should decrypt ${latestPreviewableEvent.eventId}")
+            timelineEventDecryptor.get().requestDecryption(TimelineEventDecryptor.DecryptionRequest(latestPreviewableEvent.eventId, ""))
+        }
+
        if (updateMembers) {
            val otherRoomMembers = RoomMemberHelper(realm, roomId)
                    .queryRoomMembersEvent()
--- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/timeline/DefaultTimeline.kt
+++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/timeline/DefaultTimeline.kt
@ -17,7 +17,6 @@
 package im.vector.matrix.android.internal.session.room.timeline

 import im.vector.matrix.android.api.MatrixCallback
-import im.vector.matrix.android.api.session.crypto.CryptoService
 import im.vector.matrix.android.api.session.events.model.EventType
 import im.vector.matrix.android.api.session.events.model.RelationType
 import im.vector.matrix.android.api.session.events.model.toModel
@ -73,11 +72,11 @@ internal class DefaultTimeline(
        private val taskExecutor: TaskExecutor,
        private val contextOfEventTask: GetContextOfEventTask,
        private val paginationTask: PaginationTask,
-        private val cryptoService: CryptoService,
        private val timelineEventMapper: TimelineEventMapper,
        private val settings: TimelineSettings,
        private val hiddenReadReceipts: TimelineHiddenReadReceipts,
-        private val eventBus: EventBus
+        private val eventBus: EventBus,
+        private val eventDecryptor: TimelineEventDecryptor
 ) : Timeline, TimelineHiddenReadReceipts.Delegate {

    data class OnNewTimelineEvents(val roomId: String, val eventIds: List<String>)
@ -114,8 +113,6 @@ internal class DefaultTimeline(
    override val isLive
        get() = !hasMoreToLoad(Timeline.Direction.FORWARDS)

-    private val eventDecryptor = TimelineEventDecryptor(realmConfiguration, timelineID, cryptoService)
-
    private val eventsChangeListener = OrderedRealmCollectionChangeListener<RealmResults<TimelineEventEntity>> { results, changeSet ->
        if (!results.isLoaded || !results.isValid) {
            return@OrderedRealmCollectionChangeListener
@ -607,7 +604,7 @@ internal class DefaultTimeline(

            if (timelineEvent.isEncrypted()
                    && timelineEvent.root.mxDecryptionResult == null) {
-                timelineEvent.root.eventId?.let { eventDecryptor.requestDecryption(it) }
+                timelineEvent.root.eventId?.also { eventDecryptor.requestDecryption(TimelineEventDecryptor.DecryptionRequest(it, timelineID)) }
            }

            val position = if (direction == Timeline.Direction.FORWARDS) 0 else builtEvents.size
--- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/timeline/DefaultTimelineService.kt
+++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/timeline/DefaultTimelineService.kt
@ -21,7 +21,6 @@ import androidx.lifecycle.Transformations
 import com.squareup.inject.assisted.Assisted
 import com.squareup.inject.assisted.AssistedInject
 import com.zhuinden.monarchy.Monarchy
-import im.vector.matrix.android.api.session.crypto.CryptoService
 import im.vector.matrix.android.api.session.room.timeline.Timeline
 import im.vector.matrix.android.api.session.room.timeline.TimelineEvent
 import im.vector.matrix.android.api.session.room.timeline.TimelineService
@ -41,7 +40,7 @@ internal class DefaultTimelineService @AssistedInject constructor(@Assisted priv
                                                                  private val eventBus: EventBus,
                                                                  private val taskExecutor: TaskExecutor,
                                                                  private val contextOfEventTask: GetContextOfEventTask,
-                                                                  private val cryptoService: CryptoService,
+                                                                  private val eventDecryptor: TimelineEventDecryptor,
                                                                  private val paginationTask: PaginationTask,
                                                                  private val timelineEventMapper: TimelineEventMapper,
                                                                  private val readReceiptsSummaryMapper: ReadReceiptsSummaryMapper
@ -60,11 +59,11 @@ internal class DefaultTimelineService @AssistedInject constructor(@Assisted priv
                taskExecutor = taskExecutor,
                contextOfEventTask = contextOfEventTask,
                paginationTask = paginationTask,
-                cryptoService = cryptoService,
                timelineEventMapper = timelineEventMapper,
                settings = settings,
                hiddenReadReceipts = TimelineHiddenReadReceipts(readReceiptsSummaryMapper, roomId, settings),
-                eventBus = eventBus
+                eventBus = eventBus,
+                eventDecryptor = eventDecryptor
        )
    }

--- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/timeline/TimelineEventDecryptor.kt
+++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/timeline/TimelineEventDecryptor.kt
@ -23,15 +23,19 @@ import im.vector.matrix.android.internal.crypto.model.event.EncryptedEventConten
 import im.vector.matrix.android.internal.database.mapper.asDomain
 import im.vector.matrix.android.internal.database.model.EventEntity
 import im.vector.matrix.android.internal.database.query.where
+import im.vector.matrix.android.internal.di.SessionDatabase
+import im.vector.matrix.android.internal.session.SessionScope
 import io.realm.Realm
 import io.realm.RealmConfiguration
 import timber.log.Timber
 import java.util.concurrent.ExecutorService
 import java.util.concurrent.Executors
+import javax.inject.Inject

-internal class TimelineEventDecryptor(
+@SessionScope
+internal class TimelineEventDecryptor @Inject constructor(
+        @SessionDatabase
        private val realmConfiguration: RealmConfiguration,
-        private val timelineId: String,
        private val cryptoService: CryptoService
 ) {

@ -53,9 +57,9 @@ internal class TimelineEventDecryptor(
    private var executor: ExecutorService? = null

    // Set of eventIds which are currently decrypting
-    private val existingRequests = mutableSetOf<String>()
+    private val existingRequests = mutableSetOf<DecryptionRequest>()
    // sessionId -> list of eventIds
-    private val unknownSessionsFailure = mutableMapOf<String, MutableSet<String>>()
+    private val unknownSessionsFailure = mutableMapOf<String, MutableSet<DecryptionRequest>>()

    fun start() {
        executor = Executors.newSingleThreadExecutor()
@ -74,53 +78,51 @@ internal class TimelineEventDecryptor(
        }
    }

-    fun requestDecryption(eventId: String) {
+    fun requestDecryption(request: DecryptionRequest) {
        synchronized(unknownSessionsFailure) {
-            for (eventIds in unknownSessionsFailure.values) {
-                if (eventId in eventIds) {
-                    Timber.d("Skip Decryption request for event $eventId, unknown session")
+            for (requests in unknownSessionsFailure.values) {
+                if (request in requests) {
+                    Timber.d("Skip Decryption request for event ${request.eventId}, unknown session")
                    return
                }
            }
        }
        synchronized(existingRequests) {
-            if (!existingRequests.add(eventId)) {
-                Timber.d("Skip Decryption request for event $eventId, already requested")
+            if (!existingRequests.add(request)) {
+                Timber.d("Skip Decryption request for event ${request.eventId}, already requested")
                return
            }
        }
        executor?.execute {
            Realm.getInstance(realmConfiguration).use { realm ->
-                processDecryptRequest(eventId, realm)
+                processDecryptRequest(request, realm)
            }
        }
    }

-    private fun processDecryptRequest(eventId: String, realm: Realm) {
+    private fun processDecryptRequest(request: DecryptionRequest, realm: Realm) = realm.executeTransaction {
+        val eventId = request.eventId
+        val timelineId = request.timelineId
        Timber.v("Decryption request for event $eventId")
        val eventEntity = EventEntity.where(realm, eventId = eventId).findFirst()
-                ?: return Unit.also {
+                ?: return@executeTransaction Unit.also {
                    Timber.d("Decryption request for unknown message")
                }
        val event = eventEntity.asDomain()
        try {
            val result = cryptoService.decryptEvent(event, timelineId)
            Timber.v("Successfully decrypted event $eventId")
-            realm.executeTransaction {
            eventEntity.setDecryptionResult(result)
-            }
        } catch (e: MXCryptoError) {
            Timber.w(e, "Failed to decrypt event $eventId")
            if (e is MXCryptoError.Base && e.errorType == MXCryptoError.ErrorType.UNKNOWN_INBOUND_SESSION_ID) {
                // Keep track of unknown sessions to automatically try to decrypt on new session
-                realm.executeTransaction {
                eventEntity.decryptionErrorCode = e.errorType.name
-                }
                event.content?.toModel<EncryptedEventContent>()?.let { content ->
                    content.sessionId?.let { sessionId ->
                        synchronized(unknownSessionsFailure) {
                            val list = unknownSessionsFailure.getOrPut(sessionId) { mutableSetOf() }
-                            list.add(eventId)
+                            list.add(request)
                        }
                    }
                }
@ -129,8 +131,13 @@ internal class TimelineEventDecryptor(
            Timber.e(t, "Failed to decrypt event $eventId")
        } finally {
            synchronized(existingRequests) {
-                existingRequests.remove(eventId)
+                existingRequests.remove(request)
            }
        }
    }
+
+    data class DecryptionRequest(
+            val eventId: String,
+            val timelineId: String
+    )
 }