From 9f33d732b968d5e93e9f71880cbdc2c81ee7e0b5 Mon Sep 17 00:00:00 2001 From: Toomas Vahter Date: Tue, 17 Dec 2024 11:59:45 +0200 Subject: [PATCH] Improve database model conversion performance by caching extra data and not force refreshing updated objects which affects Core Data row caches --- CHANGELOG.md | 4 +- .../StreamChat/Database/DTOs/ChannelDTO.swift | 2 +- .../Database/DTOs/CurrentUserDTO.swift | 16 ++---- .../Database/DTOs/MemberModelDTO.swift | 18 +++--- .../StreamChat/Database/DTOs/MessageDTO.swift | 39 +++++-------- .../Database/DTOs/MessageReactionDTO.swift | 19 +++---- .../StreamChat/Database/DTOs/PollDTO.swift | 13 +++-- .../Database/DTOs/PollOptionDTO.swift | 13 +++-- .../StreamChat/Database/DTOs/ThreadDTO.swift | 5 +- .../StreamChat/Database/DTOs/UserDTO.swift | 4 +- .../Database/DatabaseContainer.swift | 5 -- .../StreamChat/Utils/Codable+Extensions.swift | 57 ++++++++++++++++++- .../Utils/JSONDecoder_Tests.swift | 47 ++++++++++++++- 13 files changed, 163 insertions(+), 79 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 25bd04a097..809b2420e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). # Upcoming -### 🔄 Changed +### ⚡ Performance +- Improve performance of accessing database model properties [#3534](https://github.com/GetStream/stream-chat-swift/pull/3534) +- Improve performance of model conversions with large extra data [#3534](https://github.com/GetStream/stream-chat-swift/pull/3534) # [4.69.0](https://github.com/GetStream/stream-chat-swift/releases/tag/4.69.0) _December 18, 2024_ diff --git a/Sources/StreamChat/Database/DTOs/ChannelDTO.swift b/Sources/StreamChat/Database/DTOs/ChannelDTO.swift index bac16c3f33..08f09431dc 100644 --- a/Sources/StreamChat/Database/DTOs/ChannelDTO.swift +++ b/Sources/StreamChat/Database/DTOs/ChannelDTO.swift @@ -477,7 +477,7 @@ extension ChatChannel { let extraData: [String: RawJSON] do { - extraData = try JSONDecoder.default.decode([String: RawJSON].self, from: dto.extraData) + extraData = try JSONDecoder.stream.decodeCachedRawJSON(from: dto.extraData) } catch { log.error( "Failed to decode extra data for Channel with cid: <\(dto.cid)>, using default value instead. " diff --git a/Sources/StreamChat/Database/DTOs/CurrentUserDTO.swift b/Sources/StreamChat/Database/DTOs/CurrentUserDTO.swift index bf5446e56e..a52da2078f 100644 --- a/Sources/StreamChat/Database/DTOs/CurrentUserDTO.swift +++ b/Sources/StreamChat/Database/DTOs/CurrentUserDTO.swift @@ -224,16 +224,12 @@ extension CurrentChatUser { let user = dto.user - var extraData = [String: RawJSON]() - if !dto.user.extraData.isEmpty { - do { - extraData = try JSONDecoder.default.decode([String: RawJSON].self, from: dto.user.extraData) - } catch { - log.error( - "Failed to decode extra data for user with id: <\(dto.user.id)>, using default value instead. " - + "Error: \(error)" - ) - } + let extraData: [String: RawJSON] + do { + extraData = try JSONDecoder.stream.decodeCachedRawJSON(from: dto.user.extraData) + } catch { + log.error("Failed to decode extra data for user with id: <\(dto.user.id)>, using default value instead. Error: \(error)") + extraData = [:] } let mutedUsers: [ChatUser] = try dto.mutedUsers.map { try $0.asModel() } diff --git a/Sources/StreamChat/Database/DTOs/MemberModelDTO.swift b/Sources/StreamChat/Database/DTOs/MemberModelDTO.swift index d9666d3fc2..0ec68d89b4 100644 --- a/Sources/StreamChat/Database/DTOs/MemberModelDTO.swift +++ b/Sources/StreamChat/Database/DTOs/MemberModelDTO.swift @@ -189,7 +189,7 @@ extension ChatChannelMember { let extraData: [String: RawJSON] do { - extraData = try JSONDecoder.default.decode([String: RawJSON].self, from: dto.user.extraData) + extraData = try JSONDecoder.stream.decodeCachedRawJSON(from: dto.user.extraData) } catch { log.error( "Failed to decode extra data for user with id: <\(dto.user.id)>, using default value instead. " @@ -198,13 +198,15 @@ extension ChatChannelMember { extraData = [:] } - var memberExtraData: [String: RawJSON] = [:] - if let dtoMemberExtraData = dto.extraData { - do { - memberExtraData = try JSONDecoder.default.decode([String: RawJSON].self, from: dtoMemberExtraData) - } catch { - memberExtraData = [:] - } + let memberExtraData: [String: RawJSON] + do { + memberExtraData = try JSONDecoder.stream.decodeCachedRawJSON(from: dto.extraData) + } catch { + log.error( + "Failed to decode extra data for channel member with id: <\(dto.user.id)>, using default value instead. " + + "Error: \(error)" + ) + memberExtraData = [:] } let role = dto.channelRoleRaw.flatMap { MemberRole(rawValue: $0) } ?? .member diff --git a/Sources/StreamChat/Database/DTOs/MessageDTO.swift b/Sources/StreamChat/Database/DTOs/MessageDTO.swift index a5d43479ab..aa2891a4d7 100644 --- a/Sources/StreamChat/Database/DTOs/MessageDTO.swift +++ b/Sources/StreamChat/Database/DTOs/MessageDTO.swift @@ -1275,20 +1275,12 @@ extension MessageDTO { /// Snapshots the current state of `MessageDTO` and returns its representation for the use in API calls. func asRequestBody() -> MessageRequestBody { - var decodedExtraData: [String: RawJSON] - - if let extraData = self.extraData { - do { - decodedExtraData = try JSONDecoder.default.decode([String: RawJSON].self, from: extraData) - } catch { - log.assertionFailure( - "Failed decoding saved extra data with error: \(error). This should never happen because" - + "the extra data must be a valid JSON to be saved." - ) - decodedExtraData = [:] - } - } else { - decodedExtraData = [:] + let extraData: [String: RawJSON] + do { + extraData = try JSONDecoder.stream.decodeCachedRawJSON(from: self.extraData) + } catch { + log.assertionFailure("Failed decoding saved extra data with error: \(error). This should never happen because the extra data must be a valid JSON to be saved.") + extraData = [:] } let uploadedAttachments: [MessageAttachmentPayload] = attachments @@ -1315,7 +1307,7 @@ extension MessageDTO { pinned: pinned, pinExpires: pinExpires?.bridgeDate, pollId: poll?.id, - extraData: decodedExtraData + extraData: extraData ) } @@ -1365,17 +1357,12 @@ private extension ChatMessage { moderationDetails = dto.moderationDetails.map { MessageModerationDetails(fromDTO: $0) } textUpdatedAt = dto.textUpdatedAt?.bridgeDate - if let extraData = dto.extraData, !extraData.isEmpty { - do { - self.extraData = try JSONDecoder.default.decode([String: RawJSON].self, from: extraData) - } catch { - log - .error( - "Failed to decode extra data for Message with id: <\(dto.id)>, using default value instead. Error: \(error)" - ) - self.extraData = [:] - } - } else { + do { + extraData = try JSONDecoder.stream.decodeCachedRawJSON(from: dto.extraData) + } catch { + log.error( + "Failed to decode extra data for Message with id: <\(dto.id)>, using default value instead. Error: \(error)" + ) extraData = [:] } diff --git a/Sources/StreamChat/Database/DTOs/MessageReactionDTO.swift b/Sources/StreamChat/Database/DTOs/MessageReactionDTO.swift index ef1895b420..52181bd2f9 100644 --- a/Sources/StreamChat/Database/DTOs/MessageReactionDTO.swift +++ b/Sources/StreamChat/Database/DTOs/MessageReactionDTO.swift @@ -207,17 +207,12 @@ extension MessageReactionDTO { func asModel() throws -> ChatMessageReaction { try isNotDeleted() - let decodedExtraData: [String: RawJSON] - - if let extraData = self.extraData, !extraData.isEmpty { - do { - decodedExtraData = try JSONDecoder.default.decode([String: RawJSON].self, from: extraData) - } catch { - log.error("Failed decoding saved extra data with error: \(error)") - decodedExtraData = [:] - } - } else { - decodedExtraData = [:] + let extraData: [String: RawJSON] + do { + extraData = try JSONDecoder.stream.decodeCachedRawJSON(from: self.extraData) + } catch { + log.error("Failed decoding saved extra data with error: \(error)") + extraData = [:] } return try .init( @@ -227,7 +222,7 @@ extension MessageReactionDTO { createdAt: createdAt?.bridgeDate ?? .init(), updatedAt: updatedAt?.bridgeDate ?? .init(), author: user.asModel(), - extraData: decodedExtraData + extraData: extraData ) } } diff --git a/Sources/StreamChat/Database/DTOs/PollDTO.swift b/Sources/StreamChat/Database/DTOs/PollDTO.swift index cd7a7f1e30..295b38d412 100644 --- a/Sources/StreamChat/Database/DTOs/PollDTO.swift +++ b/Sources/StreamChat/Database/DTOs/PollDTO.swift @@ -88,11 +88,14 @@ extension PollDTO { func asModel() throws -> Poll { try isNotDeleted() - var extraData: [String: RawJSON] = [:] - if let custom, - !custom.isEmpty, - let decoded = try? JSONDecoder.default.decode([String: RawJSON].self, from: custom) { - extraData = decoded + let extraData: [String: RawJSON] + do { + extraData = try JSONDecoder.stream.decodeCachedRawJSON(from: custom) + } catch { + log.error( + "Failed to decode extra data for poll with id: <\(id)>, using default value instead. Error: \(error)" + ) + extraData = [:] } let optionsArray = (options.array as? [PollOptionDTO]) ?? [] diff --git a/Sources/StreamChat/Database/DTOs/PollOptionDTO.swift b/Sources/StreamChat/Database/DTOs/PollOptionDTO.swift index 0f277f3f33..9880226ebb 100644 --- a/Sources/StreamChat/Database/DTOs/PollOptionDTO.swift +++ b/Sources/StreamChat/Database/DTOs/PollOptionDTO.swift @@ -60,11 +60,14 @@ extension PollOptionDTO { func asModel() throws -> PollOption { try isNotDeleted() - var extraData: [String: RawJSON] = [:] - if let custom, - !custom.isEmpty, - let decoded = try? JSONDecoder.default.decode([String: RawJSON].self, from: custom) { - extraData = decoded + let extraData: [String: RawJSON] + do { + extraData = try JSONDecoder.stream.decodeCachedRawJSON(from: custom) + } catch { + log.error( + "Failed to decode extra data for poll option with id: <\(id)>, using default value instead. Error: \(error)" + ) + extraData = [:] } return PollOption( id: id, diff --git a/Sources/StreamChat/Database/DTOs/ThreadDTO.swift b/Sources/StreamChat/Database/DTOs/ThreadDTO.swift index 4cf4c7fa35..4f6991ac91 100644 --- a/Sources/StreamChat/Database/DTOs/ThreadDTO.swift +++ b/Sources/StreamChat/Database/DTOs/ThreadDTO.swift @@ -147,8 +147,11 @@ extension ThreadDTO { let extraData: [String: RawJSON] do { - extraData = try JSONDecoder.default.decode([String: RawJSON].self, from: self.extraData) + extraData = try JSONDecoder.stream.decodeCachedRawJSON(from: self.extraData) } catch { + log.error( + "Failed to decode extra data for thread with id: <\(parentMessageId)>, using default value instead. Error: \(error)" + ) extraData = [:] } diff --git a/Sources/StreamChat/Database/DTOs/UserDTO.swift b/Sources/StreamChat/Database/DTOs/UserDTO.swift index 197a4ff795..a73fd6fe57 100644 --- a/Sources/StreamChat/Database/DTOs/UserDTO.swift +++ b/Sources/StreamChat/Database/DTOs/UserDTO.swift @@ -191,7 +191,7 @@ extension UserDTO { func asRequestBody() -> UserRequestBody { let extraData: [String: RawJSON] do { - extraData = try JSONDecoder.default.decode([String: RawJSON].self, from: self.extraData) + extraData = try JSONDecoder.stream.decodeCachedRawJSON(from: self.extraData) } catch { log.assertionFailure( "Failed decoding saved extra data with error: \(error). This should never happen because" @@ -235,7 +235,7 @@ extension ChatUser { let extraData: [String: RawJSON] do { - extraData = try JSONDecoder.default.decode([String: RawJSON].self, from: dto.extraData) + extraData = try JSONDecoder.stream.decodeCachedRawJSON(from: dto.extraData) } catch { log.error( "Failed to decode extra data for user with id: <\(dto.id)>, using default value instead. " diff --git a/Sources/StreamChat/Database/DatabaseContainer.swift b/Sources/StreamChat/Database/DatabaseContainer.swift index d641a9ab19..97e8648c38 100644 --- a/Sources/StreamChat/Database/DatabaseContainer.swift +++ b/Sources/StreamChat/Database/DatabaseContainer.swift @@ -222,11 +222,6 @@ class DatabaseContainer: NSPersistentContainer, @unchecked Sendable { try actions(self.writableContext) FetchCache.clear() - // Refresh the state by merging persistent state and local state for avoiding optimistic locking failure - for object in self.writableContext.updatedObjects { - self.writableContext.refresh(object, mergeChanges: true) - } - if self.writableContext.hasChanges { log.debug("Context has changes. Saving.", subsystems: .database) try self.writableContext.save() diff --git a/Sources/StreamChat/Utils/Codable+Extensions.swift b/Sources/StreamChat/Utils/Codable+Extensions.swift index 1fd09b583e..36130fa866 100644 --- a/Sources/StreamChat/Utils/Codable+Extensions.swift +++ b/Sources/StreamChat/Utils/Codable+Extensions.swift @@ -9,6 +9,7 @@ import Foundation final class StreamJSONDecoder: JSONDecoder, @unchecked Sendable { let iso8601formatter: ISO8601DateFormatter let dateCache: NSCache + let rawJSONCache: RawJSONCache override convenience init() { let iso8601formatter = ISO8601DateFormatter() @@ -17,12 +18,21 @@ final class StreamJSONDecoder: JSONDecoder, @unchecked Sendable { let dateCache = NSCache() dateCache.countLimit = 5000 // We cache at most 5000 dates, which gives good enough performance - self.init(dateFormatter: iso8601formatter, dateCache: dateCache) + self.init( + dateFormatter: iso8601formatter, + dateCache: dateCache, + rawJSONCache: RawJSONCache(countLimit: 500) + ) } - init(dateFormatter: ISO8601DateFormatter, dateCache: NSCache) { + init( + dateFormatter: ISO8601DateFormatter, + dateCache: NSCache, + rawJSONCache: RawJSONCache + ) { iso8601formatter = dateFormatter self.dateCache = dateCache + self.rawJSONCache = rawJSONCache super.init() @@ -50,6 +60,49 @@ final class StreamJSONDecoder: JSONDecoder, @unchecked Sendable { } } +extension StreamJSONDecoder { + class RawJSONCache { + private let storage: NSCache + + init(countLimit: Int) { + storage = NSCache() + storage.countLimit = countLimit + } + + func rawJSON(forKey key: Int) -> [String: RawJSON]? { + storage.object(forKey: key as NSNumber)?.value + } + + func setRawJSON(_ value: [String: RawJSON], forKey key: Int) { + storage.setObject(BoxedRawJSON(value: value), forKey: key as NSNumber) + } + + final class BoxedRawJSON { + let value: [String: RawJSON] + + init(value: [String: RawJSON]) { + self.value = value + } + } + } + + /// A convenience method returning decoded RawJSON dictionary with caching enabled. + /// + /// Extra data stored in models can be large, what can significantly slow + /// down DTO to model conversions. This function is a convenient way for + /// caching some of the data in DTO to model conversions. + func decodeCachedRawJSON(from data: Data?) throws -> [String: RawJSON] { + guard let data, !data.isEmpty else { return [:] } + let key = data.hashValue + if let value = rawJSONCache.rawJSON(forKey: key) { + return value + } + let rawJSON = try decode([String: RawJSON].self, from: data) + rawJSONCache.setRawJSON(rawJSON, forKey: key) + return rawJSON + } +} + extension JSONDecoder { /// A default `JSONDecoder`. static let `default`: JSONDecoder = stream diff --git a/Tests/StreamChatTests/Utils/JSONDecoder_Tests.swift b/Tests/StreamChatTests/Utils/JSONDecoder_Tests.swift index ed72ee183a..d3bebf4197 100644 --- a/Tests/StreamChatTests/Utils/JSONDecoder_Tests.swift +++ b/Tests/StreamChatTests/Utils/JSONDecoder_Tests.swift @@ -148,7 +148,7 @@ final class JSONDecoder_Tests: XCTestCase { let dateFormatter = ISO8601DateFormatter_Spy() dateFormatter.formatOptions = [.withFractionalSeconds, .withInternetDateTime] let dateCache = NSCache_Spy() - let decoder = StreamJSONDecoder(dateFormatter: dateFormatter, dateCache: dateCache) + let decoder = StreamJSONDecoder(dateFormatter: dateFormatter, dateCache: dateCache, rawJSONCache: RawJSONCache_Spy()) // When we decode a payload with repeated dates let repeatedDate = "2020-06-09T08:10:40.800912Z" // If you change this, make sure to change `actualDecodedDate` below @@ -178,6 +178,33 @@ final class JSONDecoder_Tests: XCTestCase { XCTAssertEqual(value, actualDecodedDate) } } + + func test_extraDataIsCached() throws { + let extraData1: [String: RawJSON] = [ + "key": .string("value") + ] + let extraData2: [String: RawJSON] = [ + "key2": .string("value2") + ] + let data1 = try JSONEncoder().encode(extraData1) + let data2 = try JSONEncoder().encode(extraData2) + + let cacheSpy = RawJSONCache_Spy() + let decoder = StreamJSONDecoder( + dateFormatter: ISO8601DateFormatter(), + dateCache: NSCache(), + rawJSONCache: cacheSpy + ) + let decoded1 = try decoder.decodeCachedRawJSON(from: data1) + let decoded2 = try decoder.decodeCachedRawJSON(from: data2) + let decoded1Again = try decoder.decodeCachedRawJSON(from: data1) + + XCTAssertEqual(extraData1, decoded1) + XCTAssertEqual(extraData2, decoded2) + XCTAssertEqual(extraData1, decoded1Again) + XCTAssertEqual(3, cacheSpy.numberOfCalls(on: "rawJSON(forKey:)"), cacheSpy.recordedFunctions.joined(separator: " ")) + XCTAssertEqual(2, cacheSpy.numberOfCalls(on: "setRawJSON(_:forKey:)"), cacheSpy.recordedFunctions.joined(separator: " ")) + } // MARK: Helpers @@ -251,3 +278,21 @@ final class JSONDecoder_Tests: XCTestCase { } } } + +private final class RawJSONCache_Spy: StreamJSONDecoder.RawJSONCache, Spy { + let spyState = SpyState() + + init() { + super.init(countLimit: 3) + } + + override func rawJSON(forKey key: Int) -> [String: RawJSON]? { + record() + return super.rawJSON(forKey: key) + } + + override func setRawJSON(_ value: [String: RawJSON], forKey key: Int) { + record() + super.setRawJSON(value, forKey: key) + } +}