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 performance of ChatChannel database model conversions ~7 times #3325

Merged
merged 12 commits into from
Jul 25, 2024
Merged
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
# Upcoming

## StreamChat
### ⚡ Performance
- Improve performance of `ChatChannel` database model conversions more than 4 times [#3322](https://github.com/GetStream/stream-chat-swift/pull/3322)
laevandus marked this conversation as resolved.
Show resolved Hide resolved
laevandus marked this conversation as resolved.
Show resolved Hide resolved
### ✅ Added
- Expose `MissingConnectionId` + `InvalidURL` + `InvalidJSON` Errors [#3332](https://github.com/GetStream/stream-chat-swift/pull/3332)

Expand Down
103 changes: 54 additions & 49 deletions Sources/StreamChat/Database/DTOs/ChannelDTO.swift
Original file line number Diff line number Diff line change
Expand Up @@ -434,72 +434,77 @@ extension ChatChannel {
)
extraData = [:]
}


let sortedMessageDTOs = dto.messages.sorted(by: { $0.createdAt.bridgeDate > $1.createdAt.bridgeDate })
let reads: [ChatChannelRead] = try dto.reads.map { try $0.asModel() }

let unreadCount: ChannelUnreadCount = {
guard let currentUser = context.currentUser else {
guard let currentUserDTO = context.currentUser else {
return .noUnread
}

let currentUserRead = reads.first(where: { $0.user.id == currentUser.user.id })

let currentUserRead = reads.first(where: { $0.user.id == currentUserDTO.user.id })
let allUnreadMessages = currentUserRead?.unreadMessagesCount ?? 0

// Fetch count of all mentioned messages after last read
// (this is not 100% accurate but it's the best we have)
let unreadMentionsRequest = NSFetchRequest<MessageDTO>(entityName: MessageDTO.entityName)
unreadMentionsRequest.predicate = NSCompoundPredicate(andPredicateWithSubpredicates: [
MessageDTO.channelMessagesPredicate(
for: dto.cid,
deletedMessagesVisibility: context.deletedMessagesVisibility ?? .visibleForCurrentUser,
shouldShowShadowedMessages: context.shouldShowShadowedMessages ?? false
),
NSPredicate(format: "createdAt > %@", currentUserRead?.lastReadAt.bridgeDate ?? DBDate(timeIntervalSince1970: 0)),
NSPredicate(format: "%@ IN mentionedUsers", currentUser.user)
])

do {
return ChannelUnreadCount(
messages: allUnreadMessages,
mentions: try context.count(for: unreadMentionsRequest)
)
} catch {
log.error("Failed to fetch unread counts for channel `\(cid)`. Error: \(error)")
// Therefore, no unread messages with mentions and we can skip the fetch
if allUnreadMessages == 0 {
return .noUnread
}
let unreadMentionsCount = sortedMessageDTOs
.prefix(allUnreadMessages)
.filter { $0.mentionedUsers.contains(currentUserDTO.user) }
.count
return ChannelUnreadCount(
messages: allUnreadMessages,
mentions: unreadMentionsCount
)
}()

let messages: [ChatMessage] = {
MessageDTO
.load(
for: dto.cid,
limit: dto.managedObjectContext?.localCachingSettings?.chatChannel.latestMessagesLimit ?? 25,
deletedMessagesVisibility: dto.managedObjectContext?.deletedMessagesVisibility ?? .visibleForCurrentUser,
shouldShowShadowedMessages: dto.managedObjectContext?.shouldShowShadowedMessages ?? false,
context: context
)
let latestMessages: [ChatMessage] = {
var messages = sortedMessageDTOs
.prefix(dto.managedObjectContext?.localCachingSettings?.chatChannel.latestMessagesLimit ?? 25)
.compactMap { try? $0.relationshipAsModel(depth: depth) }
if let oldest = dto.oldestMessageAt?.bridgeDate {
messages = messages.filter { $0.createdAt >= oldest }
}
if let truncated = dto.truncatedAt?.bridgeDate {
messages = messages.filter { $0.createdAt >= truncated }
}
return messages
Comment on lines +464 to +470
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have some tests for these, so I kept this part to match what tests are expecting. Ideally we should add latestMessages to the ChannelDTO but I would prefer to do so in a separate PR.

}()

let latestMessageFromUser: ChatMessage? = {
guard let currentUser = context.currentUser else { return nil }

return try? MessageDTO
.loadLastMessage(
from: currentUser.user.id,
in: dto.cid,
context: context
)?
guard let currentUserId = context.currentUser?.user.id else { return nil }
return try? sortedMessageDTOs
.first(where: { messageDTO in
guard messageDTO.user.id == currentUserId else { return false }
guard messageDTO.localMessageState == nil else { return false }
return messageDTO.type != MessageType.ephemeral.rawValue
})?
.relationshipAsModel(depth: depth)
}()

let watchers = UserDTO.loadLastActiveWatchers(cid: cid, context: context)

let watchers = dto.watchers
.sorted { lhs, rhs in
let lhsActivity = lhs.lastActivityAt?.bridgeDate ?? .distantPast
let rhsActivity = rhs.lastActivityAt?.bridgeDate ?? .distantPast
if lhsActivity == rhsActivity {
return lhs.id > rhs.id
}
return lhsActivity > rhsActivity
}
.prefix(context.localCachingSettings?.chatChannel.lastActiveWatchersLimit ?? 100)
laevandus marked this conversation as resolved.
Show resolved Hide resolved
.compactMap { try? $0.asModel() }

let members = MemberDTO.loadLastActiveMembers(cid: cid, context: context)
let members = dto.members
.sorted { lhs, rhs in
let lhsActivity = lhs.user.lastActivityAt?.bridgeDate ?? .distantPast
let rhsActivity = rhs.user.lastActivityAt?.bridgeDate ?? .distantPast
if lhsActivity == rhsActivity {
return lhs.id > rhs.id
}
return lhsActivity > rhsActivity
}
.prefix(context.localCachingSettings?.chatChannel.lastActiveMembersLimit ?? 100)
.compactMap { try? $0.asModel() }

let muteDetails: MuteDetails? = {
guard let mute = dto.mute else { return nil }
return .init(
Expand Down Expand Up @@ -539,7 +544,7 @@ extension ChatChannel {
reads: reads,
cooldownDuration: Int(dto.cooldownDuration),
extraData: extraData,
latestMessages: messages,
latestMessages: latestMessages,
lastMessageFromCurrentUser: latestMessageFromUser,
pinnedMessages: pinnedMessages,
muteDetails: muteDetails,
Expand Down
11 changes: 0 additions & 11 deletions Sources/StreamChat/Database/DTOs/MemberModelDTO.swift
Original file line number Diff line number Diff line change
Expand Up @@ -92,17 +92,6 @@ extension MemberDTO {
new.id = memberId
return new
}

static func loadLastActiveMembers(cid: ChannelId, context: NSManagedObjectContext) -> [MemberDTO] {
let request = NSFetchRequest<MemberDTO>(entityName: MemberDTO.entityName)
request.predicate = NSPredicate(format: "channel.cid == %@", cid.rawValue)
request.sortDescriptors = [
ChannelMemberListSortingKey.lastActiveSortDescriptor,
ChannelMemberListSortingKey.defaultSortDescriptor
]
request.fetchLimit = context.localCachingSettings?.chatChannel.lastActiveMembersLimit ?? 100
return load(by: request, context: context)
}
}

extension NSManagedObjectContext {
Expand Down
48 changes: 22 additions & 26 deletions Sources/StreamChat/Database/DTOs/MessageDTO.swift
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ class MessageDTO: NSManagedObject {
request.predicate = NSPredicate(format: "id == %@", messageId)
return request
}

static func load(
for cid: String,
limit: Int,
Expand Down Expand Up @@ -516,19 +516,6 @@ class MessageDTO: NSManagedObject {
return (try? context.count(for: request)) ?? 0
}

static func loadLastMessage(from userId: String, in cid: String, context: NSManagedObjectContext) -> MessageDTO? {
let request = NSFetchRequest<MessageDTO>(entityName: entityName)
request.predicate = NSCompoundPredicate(andPredicateWithSubpredicates: [
channelPredicate(with: cid),
.init(format: "user.id == %@", userId),
.init(format: "type != %@", MessageType.ephemeral.rawValue),
messageSentPredicate()
])
request.sortDescriptors = [NSSortDescriptor(keyPath: \MessageDTO.createdAt, ascending: false)]
request.fetchLimit = 1
return load(by: request, context: context).first
}

static func loadSendingMessages(context: NSManagedObjectContext) -> [MessageDTO] {
let request = NSFetchRequest<MessageDTO>(entityName: MessageDTO.entityName)
request.sortDescriptors = [NSSortDescriptor(keyPath: \MessageDTO.locallyCreatedAt, ascending: false)]
Expand Down Expand Up @@ -1309,21 +1296,28 @@ private extension ChatMessage {

if let currentUser = context.currentUser {
isSentByCurrentUser = currentUser.user.id == dto.user.id
currentUserReactions = Set(
MessageReactionDTO
.loadReactions(ids: dto.ownReactions, context: context)
.compactMap { try? $0.asModel() }
)
if !dto.ownReactions.isEmpty {
currentUserReactions = Set(
MessageReactionDTO
.loadReactions(ids: dto.ownReactions, context: context)
.compactMap { try? $0.asModel() }
)
nuno-vieira marked this conversation as resolved.
Show resolved Hide resolved
} else {
currentUserReactions = []
}
nuno-vieira marked this conversation as resolved.
Show resolved Hide resolved
} else {
isSentByCurrentUser = false
currentUserReactions = []
}

latestReactions = Set(
MessageReactionDTO
.loadReactions(ids: dto.latestReactions, context: context)
.compactMap { try? $0.asModel() }
)
latestReactions = {
guard !dto.latestReactions.isEmpty else { return Set() }
return Set(
MessageReactionDTO
.loadReactions(ids: dto.latestReactions, context: context)
.compactMap { try? $0.asModel() }
)
nuno-vieira marked this conversation as resolved.
Show resolved Hide resolved
}()

threadParticipants = dto.threadParticipants.array
.compactMap { $0 as? UserDTO }
Expand All @@ -1337,8 +1331,10 @@ private extension ChatMessage {
.sorted { $0.id.index < $1.id.index }

latestReplies = {
guard !dto.replies.isEmpty else { return [] }
return MessageDTO.loadReplies(for: dto.id, limit: 5, context: context)
guard dto.replyCount > 0 else { return [] }
laevandus marked this conversation as resolved.
Show resolved Hide resolved
return dto.replies
.sorted(by: { $0.createdAt.bridgeDate > $1.createdAt.bridgeDate })
.prefix(5)
.compactMap { try? ChatMessage(fromDTO: $0, depth: depth) }
}()

Expand Down
11 changes: 0 additions & 11 deletions Sources/StreamChat/Database/DTOs/UserDTO.swift
Original file line number Diff line number Diff line change
Expand Up @@ -115,17 +115,6 @@ extension UserDTO {
new.teams = []
return new
}

static func loadLastActiveWatchers(cid: ChannelId, context: NSManagedObjectContext) -> [UserDTO] {
let request = NSFetchRequest<UserDTO>(entityName: UserDTO.entityName)
request.sortDescriptors = [
UserListSortingKey.lastActiveSortDescriptor,
UserListSortingKey.defaultSortDescriptor
]
request.predicate = NSPredicate(format: "ANY watchedChannels.cid == %@", cid.rawValue)
request.fetchLimit = context.localCachingSettings?.chatChannel.lastActiveWatchersLimit ?? 100
return load(by: request, context: context)
}
}

extension NSManagedObjectContext: UserDatabaseSession {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,6 @@ extension ChannelMemberListSortingKey {
return .init(keyPath: dateKeyPath, ascending: false)
}()

static let lastActiveSortDescriptor: NSSortDescriptor = {
let dateKeyPath: KeyPath<MemberDTO, DBDate?> = \MemberDTO.user.lastActivityAt
return .init(keyPath: dateKeyPath, ascending: false)
}()

func sortDescriptor(isAscending: Bool) -> NSSortDescriptor {
.init(key: rawValue, ascending: isAscending)
}
Expand Down
5 changes: 0 additions & 5 deletions Sources/StreamChat/Query/Sorting/UserListSortingKey.swift
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,6 @@ extension UserListSortingKey {
return .init(keyPath: stringKeyPath, ascending: false)
}()

static let lastActiveSortDescriptor: NSSortDescriptor = {
let dateKeyPath: KeyPath<UserDTO, DBDate?> = \UserDTO.lastActivityAt
return .init(keyPath: dateKeyPath, ascending: false)
}()

func sortDescriptor(isAscending: Bool) -> NSSortDescriptor? {
.init(key: rawValue, ascending: isAscending)
}
Expand Down
4 changes: 2 additions & 2 deletions Tests/StreamChatTests/Database/DTOs/ChannelDTO_Tests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1322,7 +1322,7 @@ final class ChannelDTO_Tests: XCTestCase {
XCTAssertEqual(channel.unreadCount.messages, 0)
}

func test_asModel_populatesLatestMessage() throws {
func test_asModel_populatesLatestMessage_withoutFilteringDeletedMessages() throws {
// GIVEN
database = DatabaseContainer_Spy(
kind: .inMemory,
Expand Down Expand Up @@ -1411,7 +1411,7 @@ final class ChannelDTO_Tests: XCTestCase {
// THEN
XCTAssertEqual(
Set(channel.latestMessages.map(\.id)),
Set([message1.id, deletedMessageFromCurrentUser.id, shadowedMessageFromAnotherUser.id])
Set([message1.id, deletedMessageFromCurrentUser.id, deletedMessageFromAnotherUser.id])
)
}

Expand Down
Loading