From 4818273fa7a5c9e2d52944fb868274772832a553 Mon Sep 17 00:00:00 2001 From: Allen Humphreys Date: Thu, 8 Jun 2023 15:31:11 -0400 Subject: [PATCH 1/2] Optimize schedule grouping to avoid excessive CPU and memory usage on start up --- Packages/ConfCore/ConfCore/Storage.swift | 72 +++++++++++------------- 1 file changed, 32 insertions(+), 40 deletions(-) diff --git a/Packages/ConfCore/ConfCore/Storage.swift b/Packages/ConfCore/ConfCore/Storage.swift index d0bf5cea..406d0961 100644 --- a/Packages/ConfCore/ConfCore/Storage.swift +++ b/Packages/ConfCore/ConfCore/Storage.swift @@ -95,7 +95,7 @@ public final class Storage { return } - performSerializedBackgroundWrite(writeBlock: { backgroundRealm in + performSerializedBackgroundWrite(disableAutorefresh: true, completionBlock: completion) { backgroundRealm in contentsResponse.sessions.forEach { newSession in // Replace any "unknown" resources with their full data newSession.related.filter({$0.type == RelatedResourceType.unknown.rawValue}).forEach { unknownResource in @@ -197,30 +197,22 @@ public final class Storage { // Create schedule view backgroundRealm.delete(backgroundRealm.objects(ScheduleSection.self)) - - let instances = backgroundRealm.objects(SessionInstance.self).sorted(by: SessionInstance.standardSort) - - var previousStartTime: Date? - for instance in instances { - guard instance.startTime != previousStartTime else { continue } - - autoreleasepool { - let instancesForSection = instances.filter({ $0.startTime == instance.startTime }) - - let section = ScheduleSection() - - section.representedDate = instance.startTime - section.eventIdentifier = instance.eventIdentifier - section.instances.removeAll() - section.instances.append(objectsIn: instancesForSection) - section.identifier = ScheduleSection.identifierFormatter.string(from: instance.startTime) - - backgroundRealm.add(section, update: .all) - - previousStartTime = instance.startTime - } + let instances = backgroundRealm.objects(SessionInstance.self) + + // Group all instances by common start time + // Technically, a secondary grouping on event should be used, in practice we haven't seen + // separate events that overlap in time. Someday this might hurt + Dictionary(grouping: instances, by: \.startTime).forEach { startTime, instances in + let section = ScheduleSection() + section.representedDate = startTime + section.eventIdentifier = instances[0].eventIdentifier // 0 index ok, Dictionary grouping will never give us an empty array + section.instances.removeAll() + section.instances.append(objectsIn: instances) + section.identifier = ScheduleSection.identifierFormatter.string(from: startTime) + + backgroundRealm.add(section, update: .all) } - }, disableAutorefresh: true, completionBlock: completion) + } } internal func store(liveVideosResult: Result<[SessionAsset], APIError>) { @@ -271,7 +263,7 @@ public final class Storage { return } - performSerializedBackgroundWrite(writeBlock: { backgroundRealm in + performSerializedBackgroundWrite(disableAutorefresh: true, completionBlock: completion) { backgroundRealm in let existingSections = backgroundRealm.objects(FeaturedSection.self) for section in existingSections { section.content.forEach { backgroundRealm.delete($0) } @@ -287,7 +279,7 @@ public final class Storage { content.session = backgroundRealm.object(ofType: Session.self, forPrimaryKey: content.sessionId) } } - }, disableAutorefresh: true, completionBlock: completion) + } } internal func store(configResult: Result, completion: @escaping (Error?) -> Void) { @@ -304,20 +296,20 @@ public final class Storage { return } - performSerializedBackgroundWrite(writeBlock: { backgroundRealm in + performSerializedBackgroundWrite(disableAutorefresh: false, completionBlock: completion) { backgroundRealm in // We currently only care about whatever the latest event hero is. let existingHeroData = backgroundRealm.objects(EventHero.self) backgroundRealm.delete(existingHeroData) - }, disableAutorefresh: false, completionBlock: completion) + } guard let hero = response.eventHero else { os_log("Config response didn't contain an event hero", log: self.log, type: .debug) return } - performSerializedBackgroundWrite(writeBlock: { backgroundRealm in + performSerializedBackgroundWrite(disableAutorefresh: false, completionBlock: completion) { backgroundRealm in backgroundRealm.add(hero, update: .all) - }, disableAutorefresh: false, completionBlock: completion) + } } private let serialQueue = DispatchQueue(label: "Database Serial", qos: .userInteractive) @@ -330,11 +322,11 @@ public final class Storage { /// - createTransaction: Whether the method should create its own write transaction or use the one already in place /// - notificationTokensToSkip: An array of `NotificationToken` that should not be notified when the write is committed /// - completionBlock: A block to be called when the operation is completed (called on the main queue) - internal func performSerializedBackgroundWrite(writeBlock: @escaping (Realm) throws -> Void, - disableAutorefresh: Bool = false, + internal func performSerializedBackgroundWrite(disableAutorefresh: Bool = false, createTransaction: Bool = true, notificationTokensToSkip: [NotificationToken] = [], - completionBlock: ((Error?) -> Void)? = nil) { + completionBlock: ((Error?) -> Void)? = nil, + writeBlock: @escaping (Realm) throws -> Void) { if disableAutorefresh { realm.autorefresh = false } serialQueue.async { @@ -394,13 +386,13 @@ public final class Storage { public func modify(_ object: T, with writeBlock: @escaping (T) -> Void) where T: ThreadConfined { let safeObject = ThreadSafeReference(to: object) - performSerializedBackgroundWrite(writeBlock: { backgroundRealm in + performSerializedBackgroundWrite(createTransaction: false, writeBlock: { backgroundRealm in guard let resolvedObject = backgroundRealm.resolve(safeObject) else { return } try backgroundRealm.write { writeBlock(resolvedObject) } - }, createTransaction: false) + }) } /// Gives you an opportunity to update `objects` on a background queue @@ -416,7 +408,7 @@ public final class Storage { public func modify(_ objects: [T], with writeBlock: @escaping ([T]) -> Void) where T: ThreadConfined { let safeObjects = objects.map { ThreadSafeReference(to: $0) } - performSerializedBackgroundWrite(writeBlock: { [weak self] backgroundRealm in + performSerializedBackgroundWrite(createTransaction: false, writeBlock: { [weak self] backgroundRealm in guard let self = self else { return } let resolvedObjects = safeObjects.compactMap { backgroundRealm.resolve($0) } @@ -429,7 +421,7 @@ public final class Storage { try backgroundRealm.write { writeBlock(resolvedObjects) } - }, createTransaction: false) + }) } public lazy var events: Observable> = { @@ -455,9 +447,9 @@ public final class Storage { } public func setFavorite(_ isFavorite: Bool, onSessionsWithIDs ids: [String]) { - performSerializedBackgroundWrite(writeBlock: { realm in + performSerializedBackgroundWrite(disableAutorefresh: false, createTransaction: true, writeBlock: { realm in let sessions = realm.objects(Session.self).filter(NSPredicate(format: "identifier IN %@", ids)) - + sessions.forEach { session in if isFavorite { guard !session.isFavorite else { return } @@ -466,7 +458,7 @@ public final class Storage { session.favorites.forEach { $0.isDeleted = true } } } - }, disableAutorefresh: false, createTransaction: true) + }) } public lazy var eventsObservable: Observable> = { From 4c38de2a55270d9a5f042f0a543645092e49b0e1 Mon Sep 17 00:00:00 2001 From: Allen Humphreys Date: Thu, 8 Jun 2023 20:23:49 -0400 Subject: [PATCH 2/2] Read cheap timing annotations --- Packages/ConfCore/ConfCore/Session.swift | 49 +++++++++++-------- .../ConfCore/ConfCore/SessionInstance.swift | 4 ++ Packages/ConfCore/ConfCore/Storage.swift | 33 +++++++++++++ 3 files changed, 65 insertions(+), 21 deletions(-) diff --git a/Packages/ConfCore/ConfCore/Session.swift b/Packages/ConfCore/ConfCore/Session.swift index eef9ccc7..7678637b 100644 --- a/Packages/ConfCore/ConfCore/Session.swift +++ b/Packages/ConfCore/ConfCore/Session.swift @@ -135,31 +135,38 @@ public class Session: Object, Decodable { self.assets.append(objectsIn: assets) other.related.forEach { newRelated in - let effectiveRelated: RelatedResource - - if let existingResource = realm.object(ofType: RelatedResource.self, forPrimaryKey: newRelated.identifier) { - effectiveRelated = existingResource - } else { - effectiveRelated = newRelated - } - - guard !related.contains(where: { $0.identifier == effectiveRelated.identifier }) else { return } - related.append(effectiveRelated) + realm.add(newRelated, update: .all) +// let effectiveRelated: RelatedResource +// +// if let existingResource = realm.object(ofType: RelatedResource.self, forPrimaryKey: newRelated.identifier) { +// effectiveRelated = existingResource +// } else { +// effectiveRelated = newRelated +// } +// +// guard !related.contains(where: { $0.identifier == effectiveRelated.identifier }) else { return } +// related.append(effectiveRelated) } + related.removeAll() + related.append(objectsIn: other.related) other.focuses.forEach { newFocus in - let effectiveFocus: Focus - - if let existingFocus = realm.object(ofType: Focus.self, forPrimaryKey: newFocus.name) { - effectiveFocus = existingFocus - } else { - effectiveFocus = newFocus - } - - guard !focuses.contains(where: { $0.name == effectiveFocus.name }) else { return } - - focuses.append(effectiveFocus) + realm.add(newFocus, update: .all) +// let effectiveFocus: Focus +// +// if let existingFocus = realm.object(ofType: Focus.self, forPrimaryKey: newFocus.name) { +// effectiveFocus = existingFocus +// } else { +// effectiveFocus = newFocus +// } +// +// guard !focuses.contains(where: { $0.name == effectiveFocus.name }) else { return } +// +// focuses.append(effectiveFocus) } + + focuses.removeAll() + focuses.append(objectsIn: other.focuses) } // MARK: - Decodable diff --git a/Packages/ConfCore/ConfCore/SessionInstance.swift b/Packages/ConfCore/ConfCore/SessionInstance.swift index 557a15b8..b1592a97 100644 --- a/Packages/ConfCore/ConfCore/SessionInstance.swift +++ b/Packages/ConfCore/ConfCore/SessionInstance.swift @@ -109,10 +109,14 @@ public class SessionInstance: Object, ConditionallyDecodable { eventIdentifier = other.eventIdentifier calendarEventIdentifier = other.calendarEventIdentifier + // This requires a ton of work because there are so many session instances + // And we if let otherSession = other.session, let session = session { session.merge(with: otherSession, in: realm) } + // If we collected all the keywords up front and stored them, it'd be faster than + // querying against individual sessions' keywords because you end up duplicating a lot of work let otherKeywords = other.keywords.map { newKeyword -> (Keyword) in if newKeyword.realm == nil, let existingKeyword = realm.object(ofType: Keyword.self, forPrimaryKey: newKeyword.name) { diff --git a/Packages/ConfCore/ConfCore/Storage.swift b/Packages/ConfCore/ConfCore/Storage.swift index 406d0961..05f9752c 100644 --- a/Packages/ConfCore/ConfCore/Storage.swift +++ b/Packages/ConfCore/ConfCore/Storage.swift @@ -96,6 +96,8 @@ public final class Storage { } performSerializedBackgroundWrite(disableAutorefresh: true, completionBlock: completion) { backgroundRealm in + var time = Date() + print("Starting sessions: \((Date().timeIntervalSince1970 - time.timeIntervalSince1970).formatted(.number.precision(.fractionLength(2))))") contentsResponse.sessions.forEach { newSession in // Replace any "unknown" resources with their full data newSession.related.filter({$0.type == RelatedResourceType.unknown.rawValue}).forEach { unknownResource in @@ -111,7 +113,11 @@ public final class Storage { backgroundRealm.add(newSession, update: .all) } } + print("Ending sessions: \((Date().timeIntervalSince1970 - time.timeIntervalSince1970).formatted(.number.precision(.fractionLength(2))))") + time = Date() + // TODO: Takes 8+ seconds, several notable opportunities to optimize storage accesses + print("Starting session instances: \((Date().timeIntervalSince1970 - time.timeIntervalSince1970).formatted(.number.precision(.fractionLength(2))))") // Merge existing instance data, preserving user-defined data contentsResponse.instances.forEach { newInstance in if let existingInstance = backgroundRealm.object(ofType: SessionInstance.self, forPrimaryKey: newInstance.identifier) { @@ -128,13 +134,19 @@ public final class Storage { backgroundRealm.add(newInstance, update: .all) } } + print("Ending session instances: \((Date().timeIntervalSince1970 - time.timeIntervalSince1970).formatted(.number.precision(.fractionLength(2))))") // Save everything + time = Date() + print("Starting save everything: \((Date().timeIntervalSince1970 - time.timeIntervalSince1970).formatted(.number.precision(.fractionLength(2))))") backgroundRealm.add(contentsResponse.rooms, update: .all) backgroundRealm.add(contentsResponse.tracks, update: .all) backgroundRealm.add(contentsResponse.events, update: .all) + print("Ending save everything: \((Date().timeIntervalSince1970 - time.timeIntervalSince1970).formatted(.number.precision(.fractionLength(2))))") // add instances to rooms + time = Date() + print("Starting add instances to room: \((Date().timeIntervalSince1970 - time.timeIntervalSince1970).formatted(.number.precision(.fractionLength(2))))") backgroundRealm.objects(Room.self).forEach { room in let instances = backgroundRealm.objects(SessionInstance.self).filter("roomIdentifier == %@", room.identifier) @@ -143,8 +155,12 @@ public final class Storage { room.instances.removeAll() room.instances.append(objectsIn: instances) } + print("Ending add instances to room: \((Date().timeIntervalSince1970 - time.timeIntervalSince1970).formatted(.number.precision(.fractionLength(2))))") // add instances and sessions to events + // TODO: takes 0.4 seconds, could these List's become LinkingObjects so we don't have to store them and then pull them back out? + time = Date() + print("Starting add instances and sessions to events: \((Date().timeIntervalSince1970 - time.timeIntervalSince1970).formatted(.number.precision(.fractionLength(2))))") backgroundRealm.objects(Event.self).forEach { event in let instances = backgroundRealm.objects(SessionInstance.self).filter("eventIdentifier == %@", event.identifier) let sessions = backgroundRealm.objects(Session.self).filter("eventIdentifier == %@", event.identifier) @@ -155,8 +171,12 @@ public final class Storage { event.sessions.removeAll() event.sessions.append(objectsIn: sessions) } + print("Ending add instances and sessions to events: \((Date().timeIntervalSince1970 - time.timeIntervalSince1970).formatted(.number.precision(.fractionLength(2))))") // add instances and sessions to tracks + time = Date() + print("Starting add instances and sessions to tracks: \((Date().timeIntervalSince1970 - time.timeIntervalSince1970).formatted(.number.precision(.fractionLength(2))))") + // TODO: takes 1.5 seconds, could these List's become LinkingObjects so we don't have to store them and then pull them back out? backgroundRealm.objects(Track.self).forEach { track in let instances = backgroundRealm.objects(SessionInstance.self).filter("trackIdentifier == %@", track.identifier) let sessions = backgroundRealm.objects(Session.self).filter("trackIdentifier == %@", track.identifier) @@ -173,8 +193,11 @@ public final class Storage { instance.session?.trackName = track.name } } + print("Ending add instances and sessions to tracks: \((Date().timeIntervalSince1970 - time.timeIntervalSince1970).formatted(.number.precision(.fractionLength(2))))") // add live video assets to sessions + time = Date() + print("Starting add live video assets to sessions: \((Date().timeIntervalSince1970 - time.timeIntervalSince1970).formatted(.number.precision(.fractionLength(2))))") backgroundRealm.objects(SessionAsset.self).filter("rawAssetType == %@", SessionAssetType.liveStreamVideo.rawValue).forEach { liveAsset in if let session = backgroundRealm.objects(Session.self).filter("ANY event.year == %d AND number == %@", liveAsset.year, liveAsset.sessionId).first { if !session.assets.contains(liveAsset) { @@ -182,20 +205,29 @@ public final class Storage { } } } + print("Ending add live video assets: \((Date().timeIntervalSince1970 - time.timeIntervalSince1970).formatted(.number.precision(.fractionLength(2))))") // Associate session resources with Session objects in database + time = Date() + print("Starting Associate session resources with Session objects in database: \((Date().timeIntervalSince1970 - time.timeIntervalSince1970).formatted(.number.precision(.fractionLength(2))))") backgroundRealm.objects(RelatedResource.self).filter("type == %@", RelatedResourceType.session.rawValue).forEach { resource in if let session = backgroundRealm.object(ofType: Session.self, forPrimaryKey: resource.identifier) { resource.session = session } } + print("Ending Associate session resources: \((Date().timeIntervalSince1970 - time.timeIntervalSince1970).formatted(.number.precision(.fractionLength(2))))") // Remove tracks that don't include any future session instances nor any sessions with video/live video + time = Date() + print("Starting Remove tracks that don't include any future session instances nor any sessions with video/live video: \((Date().timeIntervalSince1970 - time.timeIntervalSince1970).formatted(.number.precision(.fractionLength(2))))") let emptyTracks = backgroundRealm.objects(Track.self) .filter("SUBQUERY(sessions, $session, ANY $session.assets.rawAssetType = %@ OR ANY $session.assets.rawAssetType = %@).@count == 0", SessionAssetType.streamingVideo.rawValue, SessionAssetType.liveStreamVideo.rawValue) backgroundRealm.delete(emptyTracks) + print("Ending Remove tracks: \((Date().timeIntervalSince1970 - time.timeIntervalSince1970).formatted(.number.precision(.fractionLength(2))))") // Create schedule view + time = Date() + print("Starting Create schedule view: \((Date().timeIntervalSince1970 - time.timeIntervalSince1970).formatted(.number.precision(.fractionLength(2))))") backgroundRealm.delete(backgroundRealm.objects(ScheduleSection.self)) let instances = backgroundRealm.objects(SessionInstance.self) @@ -212,6 +244,7 @@ public final class Storage { backgroundRealm.add(section, update: .all) } + print("Ending Create schedule view: \((Date().timeIntervalSince1970 - time.timeIntervalSince1970).formatted(.number.precision(.fractionLength(2))))") } }