Improve group conversation open performance by prefetching member labels.

This commit is contained in:
Cody Henthorne
2026-03-05 12:44:19 -05:00
committed by jeffrey-signal
parent 49d3f7652d
commit 3d78d5473e
7 changed files with 112 additions and 42 deletions
@@ -171,13 +171,22 @@ object TestUsers {
return others return others
} }
fun setupGroup(): GroupId.V2 { fun setupGroup(withLabels: Boolean = false): GroupId.V2 {
val members = setupTestClients(5) val members = setupTestClients(5)
val self = Recipient.self() val self = Recipient.self()
val labels = listOf("Admin", "Mod", "VIP", "Helper", "Member")
val fullMembers = buildList { val fullMembers = buildList {
add(member(aci = self.requireAci())) add(member(aci = self.requireAci()))
addAll(members.map { member(aci = Recipient.resolved(it).requireAci()) }) addAll(
members.mapIndexed { index, id ->
if (withLabels) {
member(aci = Recipient.resolved(id).requireAci(), labelString = labels[index % labels.size])
} else {
member(aci = Recipient.resolved(id).requireAci())
}
}
)
} }
val group = DecryptedGroup( val group = DecryptedGroup(
@@ -24,6 +24,7 @@ import org.thoughtcrime.securesms.database.model.databaseprotos.BodyRangeList;
import org.thoughtcrime.securesms.groups.memberlabel.MemberLabel; import org.thoughtcrime.securesms.groups.memberlabel.MemberLabel;
import org.thoughtcrime.securesms.groups.memberlabel.MemberLabelRepository; import org.thoughtcrime.securesms.groups.memberlabel.MemberLabelRepository;
import org.thoughtcrime.securesms.recipients.Recipient; import org.thoughtcrime.securesms.recipients.Recipient;
import org.thoughtcrime.securesms.recipients.RecipientId;
import org.thoughtcrime.securesms.util.DateUtils; import org.thoughtcrime.securesms.util.DateUtils;
import org.thoughtcrime.securesms.util.MessageRecordUtil; import org.thoughtcrime.securesms.util.MessageRecordUtil;
@@ -31,6 +32,7 @@ import java.security.MessageDigest;
import java.util.Collections; import java.util.Collections;
import java.util.List; import java.util.List;
import java.util.Locale; import java.util.Locale;
import java.util.Map;
/** /**
* A view level model used to pass arbitrary message related information needed * A view level model used to pass arbitrary message related information needed
@@ -230,6 +232,25 @@ public class ConversationMessage {
@Nullable List<Mention> mentions, @Nullable List<Mention> mentions,
boolean hasBeenQuoted, boolean hasBeenQuoted,
@NonNull Recipient threadRecipient) @NonNull Recipient threadRecipient)
{
return createWithUnresolvedData(context, messageRecord, body, mentions, hasBeenQuoted, threadRecipient, null);
}
/**
* Creates a {@link ConversationMessage} wrapping the provided MessageRecord and will update and modify the provided
* mentions from placeholder to actual. This method may perform database operations to resolve mentions to display names.
*
* @param mentions List of placeholder mentions to be used to update the body in the provided MessageRecord.
* @param prefetchedLabels Pre-fetched member labels keyed by RecipientId, or null to fetch on demand.
*/
@WorkerThread
public static @NonNull ConversationMessage createWithUnresolvedData(@NonNull Context context,
@NonNull MessageRecord messageRecord,
@NonNull CharSequence body,
@Nullable List<Mention> mentions,
boolean hasBeenQuoted,
@NonNull Recipient threadRecipient,
@Nullable Map<RecipientId, MemberLabel> prefetchedLabels)
{ {
SpannableString styledAndMentionBody = null; SpannableString styledAndMentionBody = null;
MessageStyler.Result styleResult = MessageStyler.Result.none(); MessageStyler.Result styleResult = MessageStyler.Result.none();
@@ -257,8 +278,8 @@ public class ConversationMessage {
} }
FormattedDate formattedDate = getFormattedDate(context, messageRecord); FormattedDate formattedDate = getFormattedDate(context, messageRecord);
MemberLabel memberLabel = getMemberLabel(messageRecord, threadRecipient); MemberLabel memberLabel = getMemberLabel(messageRecord, threadRecipient, prefetchedLabels);
MemberLabel quoteMemberLabel = getQuoteMemberLabel(messageRecord, threadRecipient); MemberLabel quoteMemberLabel = getQuoteMemberLabel(messageRecord, threadRecipient, prefetchedLabels);
Recipient deletedBy = messageRecord.getDeletedBy() != null ? Recipient.resolved(messageRecord.getDeletedBy()) : null; Recipient deletedBy = messageRecord.getDeletedBy() != null ? Recipient.resolved(messageRecord.getDeletedBy()) : null;
return new ConversationMessage(messageRecord, return new ConversationMessage(messageRecord,
@@ -310,21 +331,30 @@ public class ConversationMessage {
} }
@WorkerThread @WorkerThread
private static @Nullable MemberLabel getMemberLabel(@NonNull MessageRecord messageRecord, @NonNull Recipient threadRecipient) { private static @Nullable MemberLabel getMemberLabel(@NonNull MessageRecord messageRecord, @NonNull Recipient threadRecipient, @Nullable Map<RecipientId, MemberLabel> prefetchedLabels) {
if (messageRecord.isOutgoing() || !threadRecipient.isPushV2Group()) { if (messageRecord.isOutgoing() || !threadRecipient.isPushV2Group()) {
return null; return null;
} }
return MemberLabelRepository.getInstance().getLabelJava(threadRecipient.requireGroupId().requireV2(), messageRecord.getFromRecipient());
if (prefetchedLabels != null) {
return prefetchedLabels.get(messageRecord.getFromRecipient().getId());
}
return MemberLabelRepository.getInstance().getLabelSync(threadRecipient.requireGroupId().requireV2(), messageRecord.getFromRecipient());
} }
@WorkerThread @WorkerThread
private static @Nullable MemberLabel getQuoteMemberLabel(@NonNull MessageRecord messageRecord, @NonNull Recipient threadRecipient) { private static @Nullable MemberLabel getQuoteMemberLabel(@NonNull MessageRecord messageRecord, @NonNull Recipient threadRecipient, @Nullable Map<RecipientId, MemberLabel> prefetchedLabels) {
if (!threadRecipient.isPushV2Group() || !(messageRecord instanceof final MmsMessageRecord mmsMessage) || mmsMessage.getQuote() == null) { if (!threadRecipient.isPushV2Group() || !(messageRecord instanceof final MmsMessageRecord mmsMessage) || mmsMessage.getQuote() == null) {
return null; return null;
} }
if (prefetchedLabels != null) {
return prefetchedLabels.get(mmsMessage.getQuote().getAuthor());
}
Recipient quoteAuthor = Recipient.resolved(mmsMessage.getQuote().getAuthor()); Recipient quoteAuthor = Recipient.resolved(mmsMessage.getQuote().getAuthor());
return MemberLabelRepository.getInstance().getLabelJava(threadRecipient.requireGroupId().requireV2(), quoteAuthor); return MemberLabelRepository.getInstance().getLabelSync(threadRecipient.requireGroupId().requireV2(), quoteAuthor);
} }
} }
} }
@@ -113,7 +113,7 @@ public final class ShowAdminsBottomSheetDialog extends BottomSheetDialogFragment
} }
List<Recipient> admins = groupRecord.getAdmins(); List<Recipient> admins = groupRecord.getAdmins();
Map<RecipientId, MemberLabel> labelsByRecipientId = MemberLabelRepository.getInstance().getLabelsJava(groupId.requireV2(), admins); Map<RecipientId, MemberLabel> labelsByRecipientId = MemberLabelRepository.getInstance().getLabelsSync(groupId.requireV2(), admins);
List<ServiceId> memberIds = groupRecord.requireV2GroupProperties().getMemberServiceIds(); List<ServiceId> memberIds = groupRecord.requireV2GroupProperties().getMemberServiceIds();
ColorizerV2 colorizer = new ColorizerV2(memberIds); ColorizerV2 colorizer = new ColorizerV2(memberIds);
@@ -118,7 +118,7 @@ class ConversationDataSource(
stopwatch.split("messages") stopwatch.split("messages")
val extraData = MessageDataFetcher.fetch(records) val extraData = MessageDataFetcher.fetch(records, threadRecipient)
stopwatch.split("extra-data") stopwatch.split("extra-data")
records = MessageDataFetcher.updateModelsWithData(records, extraData).toMutableList() records = MessageDataFetcher.updateModelsWithData(records, extraData).toMutableList()
@@ -136,7 +136,8 @@ class ConversationDataSource(
record.getDisplayBody(localContext), record.getDisplayBody(localContext),
extraData.mentionsById[record.id], extraData.mentionsById[record.id],
extraData.hasBeenQuoted.contains(record.id), extraData.hasBeenQuoted.contains(record.id),
threadRecipient threadRecipient,
extraData.memberLabels
).toMappingModel() ).toMappingModel()
} }
@@ -186,7 +187,7 @@ class ConversationDataSource(
if (record == null) { if (record == null) {
return null return null
} else { } else {
extraData = MessageDataFetcher.fetch(record) extraData = MessageDataFetcher.fetch(record, threadRecipient)
stopwatch.split("extra-data") stopwatch.split("extra-data")
record = MessageDataFetcher.updateModelWithData(record, extraData) record = MessageDataFetcher.updateModelWithData(record, extraData)
@@ -198,7 +199,8 @@ class ConversationDataSource(
record.getDisplayBody(AppDependencies.application), record.getDisplayBody(AppDependencies.application),
extraData.mentionsById[record.id], extraData.mentionsById[record.id],
extraData.hasBeenQuoted.contains(record.id), extraData.hasBeenQuoted.contains(record.id),
threadRecipient threadRecipient,
extraData.memberLabels
).toMappingModel() ).toMappingModel()
} }
} finally { } finally {
@@ -14,6 +14,7 @@ import org.thoughtcrime.securesms.database.CallTable
import org.thoughtcrime.securesms.database.SignalDatabase import org.thoughtcrime.securesms.database.SignalDatabase
import org.thoughtcrime.securesms.database.model.Mention import org.thoughtcrime.securesms.database.model.Mention
import org.thoughtcrime.securesms.database.model.MessageRecord import org.thoughtcrime.securesms.database.model.MessageRecord
import org.thoughtcrime.securesms.database.model.MmsMessageRecord
import org.thoughtcrime.securesms.database.model.ReactionRecord import org.thoughtcrime.securesms.database.model.ReactionRecord
import org.thoughtcrime.securesms.database.model.withAttachments import org.thoughtcrime.securesms.database.model.withAttachments
import org.thoughtcrime.securesms.database.model.withCall import org.thoughtcrime.securesms.database.model.withCall
@@ -21,6 +22,8 @@ import org.thoughtcrime.securesms.database.model.withPayment
import org.thoughtcrime.securesms.database.model.withPoll import org.thoughtcrime.securesms.database.model.withPoll
import org.thoughtcrime.securesms.database.model.withReactions import org.thoughtcrime.securesms.database.model.withReactions
import org.thoughtcrime.securesms.dependencies.AppDependencies import org.thoughtcrime.securesms.dependencies.AppDependencies
import org.thoughtcrime.securesms.groups.memberlabel.MemberLabel
import org.thoughtcrime.securesms.groups.memberlabel.MemberLabelRepository
import org.thoughtcrime.securesms.payments.Payment import org.thoughtcrime.securesms.payments.Payment
import org.thoughtcrime.securesms.polls.PollRecord import org.thoughtcrime.securesms.polls.PollRecord
import org.thoughtcrime.securesms.recipients.Recipient import org.thoughtcrime.securesms.recipients.Recipient
@@ -40,8 +43,8 @@ object MessageDataFetcher {
/** /**
* Singular version of [fetch]. * Singular version of [fetch].
*/ */
fun fetch(messageRecord: MessageRecord): ExtraMessageData { fun fetch(messageRecord: MessageRecord, threadRecipient: Recipient? = null): ExtraMessageData {
return fetch(listOf(messageRecord)) return fetch(listOf(messageRecord), threadRecipient)
} }
/** /**
@@ -52,7 +55,7 @@ object MessageDataFetcher {
* so this should be called on a background thread. * so this should be called on a background thread.
*/ */
@WorkerThread @WorkerThread
fun fetch(messageRecords: List<MessageRecord>): ExtraMessageData { fun fetch(messageRecords: List<MessageRecord>, threadRecipient: Recipient? = null): ExtraMessageData {
val startTimeNanos = System.nanoTime() val startTimeNanos = System.nanoTime()
val context = AppDependencies.application val context = AppDependencies.application
@@ -105,6 +108,25 @@ object MessageDataFetcher {
SignalDatabase.polls.getPollsForMessages(messageIds) SignalDatabase.polls.getPollsForMessages(messageIds)
} }
val memberLabelsFuture = if (threadRecipient != null && threadRecipient.isPushV2Group) {
executor.submitTimed {
val fromRecipients = mutableSetOf<Recipient>()
val quoteRecipientIds = mutableSetOf<RecipientId>()
for (record in messageRecords) {
if (!record.isOutgoing) {
fromRecipients.add(record.fromRecipient)
}
if (record is MmsMessageRecord && record.quote != null) {
quoteRecipientIds.add(record.quote!!.author)
}
}
val recipients = fromRecipients + Recipient.resolvedList(quoteRecipientIds)
MemberLabelRepository.instance.getLabelsSync(threadRecipient.requireGroupId().requireV2(), recipients)
}
} else {
null
}
val mentionsResult = mentionsFuture.get() val mentionsResult = mentionsFuture.get()
val hasBeenQuotedResult = hasBeenQuotedFuture.get() val hasBeenQuotedResult = hasBeenQuotedFuture.get()
val reactionsResult = reactionsFuture.get() val reactionsResult = reactionsFuture.get()
@@ -113,10 +135,11 @@ object MessageDataFetcher {
val callsResult = callsFuture.get() val callsResult = callsFuture.get()
val recipientsResult = recipientsFuture.get() val recipientsResult = recipientsFuture.get()
val pollsResult = pollsFuture.get() val pollsResult = pollsFuture.get()
val memberLabelsResult = memberLabelsFuture?.get()
val wallTimeMs = (System.nanoTime() - startTimeNanos).nanoseconds.toDouble(DurationUnit.MILLISECONDS) val wallTimeMs = (System.nanoTime() - startTimeNanos).nanoseconds.toDouble(DurationUnit.MILLISECONDS)
val cpuTimeNanos = arrayOf(mentionsResult, hasBeenQuotedResult, reactionsResult, attachmentsResult, paymentsResult, callsResult, recipientsResult).sumOf { it.durationNanos } val cpuTimeNanos = arrayOf(mentionsResult, hasBeenQuotedResult, reactionsResult, attachmentsResult, paymentsResult, callsResult, recipientsResult).sumOf { it.durationNanos } + (memberLabelsResult?.durationNanos ?: 0)
val cpuTimeMs = cpuTimeNanos.nanoseconds.toDouble(DurationUnit.MILLISECONDS) val cpuTimeMs = cpuTimeNanos.nanoseconds.toDouble(DurationUnit.MILLISECONDS)
return ExtraMessageData( return ExtraMessageData(
@@ -127,7 +150,8 @@ object MessageDataFetcher {
payments = paymentsResult.result, payments = paymentsResult.result,
calls = callsResult.result, calls = callsResult.result,
polls = pollsResult.result, polls = pollsResult.result,
timeLog = "mentions: ${mentionsResult.duration}, is-quoted: ${hasBeenQuotedResult.duration}, reactions: ${reactionsResult.duration}, attachments: ${attachmentsResult.duration}, payments: ${paymentsResult.duration}, calls: ${callsResult.duration} >> cpuTime: ${cpuTimeMs.roundedString(2)}, wallTime: ${wallTimeMs.roundedString(2)}" memberLabels = memberLabelsResult?.result,
timeLog = "mentions: ${mentionsResult.duration}, is-quoted: ${hasBeenQuotedResult.duration}, reactions: ${reactionsResult.duration}, attachments: ${attachmentsResult.duration}, payments: ${paymentsResult.duration}, calls: ${callsResult.duration}, member-labels: ${memberLabelsResult?.duration ?: "n/a"} >> cpuTime: ${cpuTimeMs.roundedString(2)}, wallTime: ${wallTimeMs.roundedString(2)}"
) )
} }
@@ -200,6 +224,7 @@ object MessageDataFetcher {
val payments: Map<Long, Payment>, val payments: Map<Long, Payment>,
val calls: Map<Long, CallTable.Call>, val calls: Map<Long, CallTable.Call>,
val polls: Map<Long, PollRecord>, val polls: Map<Long, PollRecord>,
val memberLabels: Map<RecipientId, MemberLabel>?,
val timeLog: String val timeLog: String
) )
} }
@@ -8,7 +8,6 @@ package org.thoughtcrime.securesms.groups.memberlabel
import android.content.Context import android.content.Context
import androidx.annotation.WorkerThread import androidx.annotation.WorkerThread
import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.runBlocking
import kotlinx.coroutines.withContext import kotlinx.coroutines.withContext
import org.signal.core.models.ServiceId import org.signal.core.models.ServiceId
import org.signal.core.util.orNull import org.signal.core.util.orNull
@@ -54,25 +53,42 @@ class MemberLabelRepository private constructor(
* Gets the member label for a specific recipient in the group (blocking version for Java compatibility). * Gets the member label for a specific recipient in the group (blocking version for Java compatibility).
*/ */
@WorkerThread @WorkerThread
fun getLabelJava(groupId: GroupId.V2, recipient: Recipient): MemberLabel? = runBlocking { getLabel(groupId, recipient) } fun getLabelSync(groupId: GroupId.V2, recipient: Recipient): MemberLabel? {
if (!RemoteConfig.receiveMemberLabels) {
return null
}
val aci = recipient.serviceId.orNull() as? ServiceId.ACI ?: return null
val groupRecord = groupsTable.getGroup(groupId).orNull() ?: return null
return groupRecord.requireV2GroupProperties().memberLabel(aci)?.sanitized()
}
/** /**
* Gets member labels for a list of recipients in a group (blocking version for Java compatibility). * Gets member labels for a list of recipients in a group (blocking version for Java compatibility).
*/ */
@WorkerThread @WorkerThread
fun getLabelsJava(groupId: GroupId.V2, recipients: List<Recipient>): Map<RecipientId, MemberLabel> = runBlocking { getLabels(groupId, recipients) } fun getLabelsSync(groupId: GroupId.V2, recipients: Collection<Recipient>): Map<RecipientId, MemberLabel> {
if (!RemoteConfig.receiveMemberLabels) {
return emptyMap()
}
val groupRecord = groupsTable.getGroup(groupId).orNull() ?: return emptyMap()
val labelsByAci = groupRecord.requireV2GroupProperties().memberLabelsByAci()
return buildMap {
recipients.forEach { recipient ->
val aci = recipient.serviceId.orNull() as? ServiceId.ACI
labelsByAci[aci]?.let { label -> put(recipient.id, label.sanitized()) }
}
}
}
/** /**
* Gets the member label for a specific recipient in the group. * Gets the member label for a specific recipient in the group.
*/ */
suspend fun getLabel(groupId: GroupId.V2, recipient: Recipient): MemberLabel? = withContext(Dispatchers.IO) { suspend fun getLabel(groupId: GroupId.V2, recipient: Recipient): MemberLabel? = withContext(Dispatchers.IO) {
if (!RemoteConfig.receiveMemberLabels) { getLabelSync(groupId, recipient)
return@withContext null
}
val aci = recipient.serviceId.orNull() as? ServiceId.ACI ?: return@withContext null
val groupRecord = groupsTable.getGroup(groupId).orNull() ?: return@withContext null
groupRecord.requireV2GroupProperties().memberLabel(aci)?.sanitized()
} }
/** /**
@@ -81,19 +97,7 @@ class MemberLabelRepository private constructor(
* Returns a map of [RecipientId] to [MemberLabel] for members that have labels. * Returns a map of [RecipientId] to [MemberLabel] for members that have labels.
*/ */
suspend fun getLabels(groupId: GroupId.V2, recipients: List<Recipient>): Map<RecipientId, MemberLabel> = withContext(Dispatchers.IO) { suspend fun getLabels(groupId: GroupId.V2, recipients: List<Recipient>): Map<RecipientId, MemberLabel> = withContext(Dispatchers.IO) {
if (!RemoteConfig.receiveMemberLabels) { getLabelsSync(groupId, recipients)
return@withContext emptyMap()
}
val groupRecord = groupsTable.getGroup(groupId).orNull() ?: return@withContext emptyMap()
val labelsByAci = groupRecord.requireV2GroupProperties().memberLabelsByAci()
buildMap {
recipients.forEach { recipient ->
val aci = recipient.serviceId.orNull() as? ServiceId.ACI
labelsByAci[aci]?.let { label -> put(recipient.id, label.sanitized()) }
}
}
} }
/** /**
@@ -137,7 +137,7 @@ final class RecipientDialogViewModel extends ViewModel {
if (groupId != null && groupId.isV2() && recipient.isIndividual() && !recipient.isSelf()) { if (groupId != null && groupId.isV2() && recipient.isIndividual() && !recipient.isSelf()) {
SignalExecutors.BOUNDED.execute(() -> { SignalExecutors.BOUNDED.execute(() -> {
GroupId.V2 v2GroupId = (GroupId.V2) groupId; GroupId.V2 v2GroupId = (GroupId.V2) groupId;
MemberLabel label = MemberLabelRepository.getInstance().getLabelJava(v2GroupId, recipient); MemberLabel label = MemberLabelRepository.getInstance().getLabelSync(v2GroupId, recipient);
StyledMemberLabel styledLabel = null; StyledMemberLabel styledLabel = null;
if (label != null) { if (label != null) {