Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve usage of realm #5297

Merged
merged 4 commits into from
Feb 22, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,7 @@

package org.matrix.android.sdk.api.session.room.model

import org.matrix.android.sdk.api.session.user.model.User

data class ReadReceipt(
val user: User,
val roomMember: RoomMemberSummary,
val originServerTs: Long
)
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,9 @@ import com.zhuinden.monarchy.Monarchy
import io.realm.Realm
import io.realm.RealmConfiguration
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.asCoroutineDispatcher
import kotlinx.coroutines.isActive
import kotlinx.coroutines.launch
import kotlinx.coroutines.sync.Semaphore
import kotlinx.coroutines.sync.withPermit
import kotlinx.coroutines.withContext
import timber.log.Timber

Expand All @@ -37,30 +35,26 @@ internal fun <T> CoroutineScope.asyncTransaction(realmConfiguration: RealmConfig
}
}

private val realmSemaphore = Semaphore(1)

suspend fun <T> awaitTransaction(config: RealmConfiguration, transaction: suspend (realm: Realm) -> T): T {
return realmSemaphore.withPermit {
withContext(Dispatchers.IO) {
Realm.getInstance(config).use { bgRealm ->
bgRealm.beginTransaction()
val result: T
try {
val start = System.currentTimeMillis()
result = transaction(bgRealm)
if (isActive) {
bgRealm.commitTransaction()
val end = System.currentTimeMillis()
val time = end - start
Timber.v("Execute transaction in $time millis")
}
} finally {
if (bgRealm.isInTransaction) {
bgRealm.cancelTransaction()
}
return withContext(Realm.WRITE_EXECUTOR.asCoroutineDispatcher()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it worth holding onto the Realm.WRITE_EXECUTOR.asCoroutineDispatcher() instance?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so, at least realm is not doing that neither

Realm.getInstance(config).use { bgRealm ->
bgRealm.beginTransaction()
val result: T
try {
val start = System.currentTimeMillis()
result = transaction(bgRealm)
if (isActive) {
bgRealm.commitTransaction()
val end = System.currentTimeMillis()
val time = end - start
Timber.v("Execute transaction in $time millis")
}
} finally {
if (bgRealm.isInTransaction) {
bgRealm.cancelTransaction()
}
result
}
result
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ package org.matrix.android.sdk.internal.database.mapper
import org.matrix.android.sdk.api.session.room.model.ReadReceipt
import org.matrix.android.sdk.internal.database.RealmSessionProvider
import org.matrix.android.sdk.internal.database.model.ReadReceiptsSummaryEntity
import org.matrix.android.sdk.internal.database.model.UserEntity
import org.matrix.android.sdk.internal.database.model.RoomMemberSummaryEntity
import org.matrix.android.sdk.internal.database.query.where
import javax.inject.Inject

Expand All @@ -29,14 +29,12 @@ internal class ReadReceiptsSummaryMapper @Inject constructor(private val realmSe
if (readReceiptsSummaryEntity == null) {
return emptyList()
}
return realmSessionProvider.withRealm { realm ->
val readReceipts = readReceiptsSummaryEntity.readReceipts
readReceipts
.mapNotNull {
val user = UserEntity.where(realm, it.userId).findFirst()
?: return@mapNotNull null
ReadReceipt(user.asDomain(), it.originServerTs.toLong())
}
}
val readReceipts = readReceiptsSummaryEntity.readReceipts
return readReceipts
.mapNotNull {
val roomMember = RoomMemberSummaryEntity.where(readReceiptsSummaryEntity.realm, roomId = it.roomId, userId = it.userId).findFirst()
?: return@mapNotNull null
ReadReceipt(roomMember.asDomain(), it.originServerTs.toLong())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are there any side effects from switching User to RoomMember?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

User avatar can be set per room, so maybe there will be a visible effect.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No this should just fix some cases where user has changed avatar/name only in the room

}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ internal class TimelineEventMapper @Inject constructor(private val readReceiptsS
),
readReceipts = readReceipts
?.distinctBy {
it.user
it.roomMember
}?.sortedByDescending {
it.originServerTs
}.orEmpty()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import org.matrix.android.sdk.api.session.room.model.Membership
import org.matrix.android.sdk.api.session.room.model.create.CreateRoomParams
import org.matrix.android.sdk.api.session.room.model.create.CreateRoomPreset
import org.matrix.android.sdk.internal.database.awaitNotEmptyResult
import org.matrix.android.sdk.internal.database.awaitTransaction
import org.matrix.android.sdk.internal.database.model.RoomSummaryEntity
import org.matrix.android.sdk.internal.database.model.RoomSummaryEntityFields
import org.matrix.android.sdk.internal.database.query.where
Expand Down Expand Up @@ -105,7 +106,7 @@ internal class DefaultCreateRoomTask @Inject constructor(
throw CreateRoomFailure.CreatedWithTimeout(roomId)
}

Realm.getInstance(realmConfiguration).executeTransactionAsync {
awaitTransaction(realmConfiguration){
RoomSummaryEntity.where(it, roomId).findFirst()?.lastActivityTime = System.currentTimeMillis()
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import org.matrix.android.sdk.api.session.room.failure.JoinRoomFailure
import org.matrix.android.sdk.api.session.room.members.ChangeMembershipState
import org.matrix.android.sdk.api.session.room.model.Membership
import org.matrix.android.sdk.internal.database.awaitNotEmptyResult
import org.matrix.android.sdk.internal.database.awaitTransaction
import org.matrix.android.sdk.internal.database.model.RoomSummaryEntity
import org.matrix.android.sdk.internal.database.model.RoomSummaryEntityFields
import org.matrix.android.sdk.internal.database.query.where
Expand Down Expand Up @@ -89,11 +90,9 @@ internal class DefaultJoinRoomTask @Inject constructor(
} catch (exception: TimeoutCancellationException) {
throw JoinRoomFailure.JoinedWithTimeout
}

Realm.getInstance(realmConfiguration).executeTransactionAsync {
awaitTransaction(realmConfiguration){
RoomSummaryEntity.where(it, roomId).findFirst()?.lastActivityTime = System.currentTimeMillis()
}

setReadMarkers(roomId)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,7 @@ internal class TimelineChunk(private val chunkEntity: ChunkEntity,
private val timelineEventsChangeListener =
OrderedRealmCollectionChangeListener { results: RealmResults<TimelineEventEntity>, changeSet: OrderedCollectionChangeSet ->
Timber.v("on timeline events chunk update")
val frozenResults = results.freeze()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤞

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the main fix ^ =P

handleDatabaseChangeSet(frozenResults, changeSet)
handleDatabaseChangeSet(results, changeSet)
}

private var timelineEventEntities: RealmResults<TimelineEventEntity> = chunkEntity.sortedTimelineEvents(timelineSettings.rootThreadEventId)
Expand Down Expand Up @@ -287,7 +286,7 @@ internal class TimelineChunk(private val chunkEntity: ChunkEntity,
* @return the number of events loaded. If we are in a thread timeline it also returns
* whether or not we reached the end/root message
*/
private suspend fun loadFromStorage(count: Int, direction: Timeline.Direction): LoadedFromStorage {
private fun loadFromStorage(count: Int, direction: Timeline.Direction): LoadedFromStorage {
val displayIndex = getNextDisplayIndex(direction) ?: return LoadedFromStorage()
val baseQuery = timelineEventEntities.where()

Expand Down Expand Up @@ -428,10 +427,10 @@ internal class TimelineChunk(private val chunkEntity: ChunkEntity,
* This method is responsible for managing insertions and updates of events on this chunk.
*
*/
private fun handleDatabaseChangeSet(frozenResults: RealmResults<TimelineEventEntity>, changeSet: OrderedCollectionChangeSet) {
private fun handleDatabaseChangeSet(results: RealmResults<TimelineEventEntity>, changeSet: OrderedCollectionChangeSet) {
val insertions = changeSet.insertionRanges
for (range in insertions) {
val newItems = frozenResults
val newItems = results
.subList(range.startIndex, range.startIndex + range.length)
.map { it.buildAndDecryptIfNeeded() }
builtEventsIndexes.entries.filter { it.value >= range.startIndex }.forEach { it.setValue(it.value + range.length) }
Expand All @@ -447,7 +446,7 @@ internal class TimelineChunk(private val chunkEntity: ChunkEntity,
val modifications = changeSet.changeRanges
for (range in modifications) {
for (modificationIndex in (range.startIndex until range.startIndex + range.length)) {
val updatedEntity = frozenResults[modificationIndex] ?: continue
val updatedEntity = results[modificationIndex] ?: continue
try {
builtEvents[modificationIndex] = updatedEntity.buildAndDecryptIfNeeded()
} catch (failure: Throwable) {
Expand All @@ -458,20 +457,20 @@ internal class TimelineChunk(private val chunkEntity: ChunkEntity,
if (insertions.isNotEmpty() || modifications.isNotEmpty()) {
onBuiltEvents(true)
}

}

private fun getNextDisplayIndex(direction: Timeline.Direction): Int? {
val frozenTimelineEvents = timelineEventEntities.freeze()
if (frozenTimelineEvents.isEmpty()) {
if (timelineEventEntities.isEmpty()) {
return null
}
return if (builtEvents.isEmpty()) {
if (initialEventId != null) {
frozenTimelineEvents.where().equalTo(TimelineEventEntityFields.EVENT_ID, initialEventId).findFirst()?.displayIndex
timelineEventEntities.where().equalTo(TimelineEventEntityFields.EVENT_ID, initialEventId).findFirst()?.displayIndex
} else if (direction == Timeline.Direction.BACKWARDS) {
frozenTimelineEvents.first(null)?.displayIndex
timelineEventEntities.first(null)?.displayIndex
} else {
frozenTimelineEvents.last(null)?.displayIndex
timelineEventEntities.last(null)?.displayIndex
}
} else if (direction == Timeline.Direction.FORWARDS) {
builtEvents.first().displayIndex + 1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,7 @@ class TimelineEventController @Inject constructor(private val dateFormatter: Vec
val event = itr.previous()
timelineEventsGroups.addOrIgnore(event)
val currentReadReceipts = ArrayList(event.readReceipts).filter {
it.user.userId != session.myUserId
it.roomMember.userId != session.myUserId
}
if (timelineEventVisibilityHelper.shouldShowEvent(
timelineEvent = event,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class ReadReceiptsItemFactory @Inject constructor(private val avatarRenderer: Av
}
val readReceiptsData = readReceipts
.map {
ReadReceiptData(it.user.userId, it.user.avatarUrl, it.user.displayName, it.originServerTs)
ReadReceiptData(it.roomMember.userId, it.roomMember.avatarUrl, it.roomMember.displayName, it.originServerTs)
}
.toList()

Expand Down