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.

Sign up for GitHub

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 and ChatMessage equality #3335

Merged
merged 2 commits into from
Jul 25, 2024
Merged

Improve performance of ChatChannel and ChatMessage equality #3335

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3347e00
Improve performance of ChatChannel and ChatMessage equality which cau…
laevandus Jul 25, 2024
fdafddd
Merge branch 'develop' into perf/swiftui-list-updates
laevandus Jul 25, 2024
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
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` and `ChatMessage` equality checks [#3335](https://github.com/GetStream/stream-chat-swift/pull/3335)
### ✅ Added
- Expose `MissingConnectionId` + `InvalidURL` + `InvalidJSON` Errors [#3332](https://github.com/GetStream/stream-chat-swift/pull/3332)

Expand Down
43 changes: 22 additions & 21 deletions Sources/StreamChat/Models/Channel.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,27 +237,28 @@ extension ChatChannel: AnyChannel {}

extension ChatChannel: Hashable {
public static func == (lhs: ChatChannel, rhs: ChatChannel) -> Bool {
lhs.cid == rhs.cid &&
lhs.updatedAt == rhs.updatedAt &&
lhs.cooldownDuration == rhs.cooldownDuration &&
lhs.createdAt == rhs.createdAt &&
lhs.createdBy == rhs.createdBy &&
lhs.deletedAt == rhs.deletedAt &&
lhs.extraData == rhs.extraData &&
lhs.imageURL == rhs.imageURL &&
lhs.isFrozen == rhs.isFrozen &&
lhs.isHidden == rhs.isHidden &&
lhs.lastMessageAt == rhs.lastMessageAt &&
lhs.memberCount == rhs.memberCount &&
lhs.membership == rhs.membership &&
lhs.muteDetails == rhs.muteDetails &&
lhs.name == rhs.name &&
lhs.ownCapabilities == rhs.ownCapabilities &&
lhs.previewMessage == rhs.previewMessage &&
lhs.reads == rhs.reads &&
lhs.team == rhs.team &&
lhs.truncatedAt == rhs.truncatedAt &&
lhs.watcherCount == rhs.watcherCount
guard lhs.cid == rhs.cid else { return false }
guard lhs.updatedAt == rhs.updatedAt else { return false }
guard lhs.lastMessageAt == rhs.lastMessageAt else { return false }
guard lhs.muteDetails == rhs.muteDetails else { return false }
guard lhs.reads == rhs.reads else { return false }
guard lhs.previewMessage == rhs.previewMessage else { return false }
guard lhs.name == rhs.name else { return false }
guard lhs.watcherCount == rhs.watcherCount else { return false }
Copy link
Contributor

Choose a reason for hiding this comment

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

I see you also changed the order, did that have an impact?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Slightly re-ordered them depending on my gut feeling what could change more often. Nothing really noticeable. I have seen ownCapabilities being the most visible.

Comment on lines +240 to +247
Copy link
Member

@nuno-vieira nuno-vieira Jul 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not sure I fully understood the issue 🤔 Isn't this the same code?

Changing the == to use guard let takes away a lot of this overhead.

What it is different from not using a guard?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems like compiler produces slightly different code where now it does not copy channels and messages so much as it used to be with a long expression.

guard lhs.createdAt == rhs.createdAt else { return false }
guard lhs.cooldownDuration == rhs.cooldownDuration else { return false }
guard lhs.createdBy == rhs.createdBy else { return false }
guard lhs.deletedAt == rhs.deletedAt else { return false }
guard lhs.extraData == rhs.extraData else { return false }
guard lhs.imageURL == rhs.imageURL else { return false }
guard lhs.isFrozen == rhs.isFrozen else { return false }
guard lhs.isHidden == rhs.isHidden else { return false }
guard lhs.memberCount == rhs.memberCount else { return false }
guard lhs.membership == rhs.membership else { return false }
guard lhs.team == rhs.team else { return false }
guard lhs.truncatedAt == rhs.truncatedAt else { return false }
guard lhs.ownCapabilities == rhs.ownCapabilities else { return false }
return true
}

public func hash(into hasher: inout Hasher) {
Expand Down
47 changes: 24 additions & 23 deletions Sources/StreamChat/Models/ChatMessage.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,29 +359,30 @@ public extension ChatMessage {

extension ChatMessage: Hashable {
public static func == (lhs: Self, rhs: Self) -> Bool {
lhs.id == rhs.id &&
lhs.updatedAt == rhs.updatedAt &&
lhs.allAttachments == rhs.allAttachments &&
lhs.arguments == rhs.arguments &&
lhs.author == rhs.author &&
lhs.command == rhs.command &&
lhs.currentUserReactionsCount == rhs.currentUserReactionsCount &&
lhs.extraData == rhs.extraData &&
lhs.isFlaggedByCurrentUser == rhs.isFlaggedByCurrentUser &&
lhs.isShadowed == rhs.isShadowed &&
lhs.localState == rhs.localState &&
lhs.parentMessageId == rhs.parentMessageId &&
lhs.quotedMessage == rhs.quotedMessage &&
lhs.reactionCounts == rhs.reactionCounts &&
lhs.reactionGroups == rhs.reactionGroups &&
lhs.reactionScores == rhs.reactionScores &&
lhs.readByCount == rhs.readByCount &&
lhs.replyCount == rhs.replyCount &&
lhs.showReplyInChannel == rhs.showReplyInChannel &&
lhs.text == rhs.text &&
lhs.threadParticipantsCount == rhs.threadParticipantsCount &&
lhs.translations == rhs.translations &&
lhs.type == rhs.type
guard lhs.id == rhs.id else { return false }
guard lhs.localState == rhs.localState else { return false }
guard lhs.updatedAt == rhs.updatedAt else { return false }
guard lhs.allAttachments == rhs.allAttachments else { return false }
guard lhs.author == rhs.author else { return false }
guard lhs.currentUserReactionsCount == rhs.currentUserReactionsCount else { return false }
guard lhs.text == rhs.text else { return false }
guard lhs.parentMessageId == rhs.parentMessageId else { return false }
guard lhs.reactionCounts == rhs.reactionCounts else { return false }
guard lhs.reactionGroups == rhs.reactionGroups else { return false }
guard lhs.reactionScores == rhs.reactionScores else { return false }
guard lhs.readByCount == rhs.readByCount else { return false }
guard lhs.replyCount == rhs.replyCount else { return false }
guard lhs.showReplyInChannel == rhs.showReplyInChannel else { return false }
guard lhs.threadParticipantsCount == rhs.threadParticipantsCount else { return false }
guard lhs.arguments == rhs.arguments else { return false }
guard lhs.command == rhs.command else { return false }
guard lhs.extraData == rhs.extraData else { return false }
guard lhs.isFlaggedByCurrentUser == rhs.isFlaggedByCurrentUser else { return false }
guard lhs.isShadowed == rhs.isShadowed else { return false }
guard lhs.quotedMessage == rhs.quotedMessage else { return false }
guard lhs.translations == rhs.translations else { return false }
guard lhs.type == rhs.type else { return false }
return true
}

public func hash(into hasher: inout Hasher) {
Expand Down
Loading