From b79c8af12c2294725159a77cbcd1387d8abd8f4f Mon Sep 17 00:00:00 2001 From: Shadowfacts Date: Thu, 9 Jun 2022 23:28:34 -0400 Subject: [PATCH] Refetch everything when there are inserted items, performance with trying to merge change was just too bad --- .../Screens/Items/ItemsViewController.swift | 106 +++++++----------- 1 file changed, 40 insertions(+), 66 deletions(-) diff --git a/Reader/Screens/Items/ItemsViewController.swift b/Reader/Screens/Items/ItemsViewController.swift index 6d4e8ee..d7f77b0 100644 --- a/Reader/Screens/Items/ItemsViewController.swift +++ b/Reader/Screens/Items/ItemsViewController.swift @@ -75,21 +75,9 @@ class ItemsViewController: UIViewController { ]) fetchRequest.sortDescriptors = [NSSortDescriptor(key: "published", ascending: false)] - do { - let ids = try fervorController.persistentContainer.viewContext.fetch(fetchRequest) - - var snapshot = NSDiffableDataSourceSnapshot() - snapshot.appendSections([.items]) - snapshot.appendItems(ids, toSection: .items) - dataSource.apply(snapshot, animatingDifferences: false) - - NotificationCenter.default.addObserver(self, selector: #selector(managedObjectsDidChange), name: .NSManagedObjectContextObjectsDidChange, object: fervorController.persistentContainer.viewContext) - - } catch { - let alert = UIAlertController(title: "Error fetching items", message: error.localizedDescription, preferredStyle: .alert) - alert.addAction(UIAlertAction(title: "Ok", style: .default, handler: nil)) - present(alert, animated: true) - } + fetchItems() + + NotificationCenter.default.addObserver(self, selector: #selector(managedObjectsDidChange), name: .NSManagedObjectContextObjectsDidChange, object: fervorController.persistentContainer.viewContext) } private func createDataSource() -> UICollectionViewDiffableDataSource { @@ -104,72 +92,58 @@ class ItemsViewController: UIViewController { } } + private func fetchItems() { + do { + let ids = try fervorController.persistentContainer.viewContext.fetch(fetchRequest) + + var snapshot = NSDiffableDataSourceSnapshot() + snapshot.appendSections([.items]) + snapshot.appendItems(ids, toSection: .items) + dataSource.apply(snapshot, animatingDifferences: false) + + } catch { + let alert = UIAlertController(title: "Error fetching items", message: error.localizedDescription, preferredStyle: .alert) + alert.addAction(UIAlertAction(title: "Ok", style: .default, handler: nil)) + present(alert, animated: true) + } + } + + private var updateId = 0 + @objc private func managedObjectsDidChange(_ notification: Notification) { + let id = updateId + updateId += 1 + print("\(id) Managed objects did change") let inserted = notification.userInfo?[NSInsertedObjectsKey] as? NSSet let updated = notification.userInfo?[NSUpdatedObjectsKey] as? NSSet let deleted = notification.userInfo?[NSDeletedObjectsKey] as? NSSet - // managed objectss from the notification are tied to the thread it was delivered on - // so get the published dates and evaluate the predicate here - let insertedItems = inserted?.compactMap { $0 as? Item }.filter { fetchRequest.predicate?.evaluate(with: $0) ?? true }.map { ($0, $0.published) } + let hasInsertedItems = inserted?.lazy.compactMap { $0 as? Item }.contains { fetchRequest.predicate?.evaluate(with: $0) ?? true } ?? false + + if hasInsertedItems { + print("\(id) Has inserted items, skipping merge") + // if any items were inserted, just refetch everything. it's more expensive than it's worth to try and merge the changes into the current snapshot + self.fetchItems() + return + } var snapshot = self.dataSource.snapshot() + // the itemIdentifiers getter takes a lot of time in profiles, so only call the getter once + let snapshotItems = snapshot.itemIdentifiers if let updated = updated { - let knownUpdated = updated.compactMap { ($0 as? Item)?.objectID }.filter { snapshot.itemIdentifiers.contains($0) } + print("\(id) Updated: \(updated.count)") + let knownUpdated = updated.compactMap { ($0 as? Item)?.objectID }.filter { snapshotItems.contains($0) } snapshot.reconfigureItems(knownUpdated) } if let deleted = deleted { - let knownDeleted = deleted.compactMap { ($0 as? Item)?.objectID }.filter { snapshot.itemIdentifiers.contains($0) } + print("\(id) Deleted: \(deleted.count)") + let knownDeleted = deleted.compactMap { ($0 as? Item)?.objectID }.filter { snapshotItems.contains($0) } snapshot.deleteItems(knownDeleted) } - if let insertedItems = insertedItems { - self.fervorController.persistentContainer.performBackgroundTask { ctx in - - // for newly inserted items, the ctx doesn't have the published date so we check the data we got from the notification - func publishedForItem(_ id: NSManagedObjectID) -> Date? { - if let (_, pub) = insertedItems.first(where: { $0.0.objectID == id }) { - return pub - } else { - return (ctx.object(with: id) as! Item).published - } - } - - // todo: this feels inefficient - for (inserted, insertedPublished) in insertedItems { - // todo: uhh, what does sql do if the published date is nil? - guard let insertedPublished = insertedPublished else { - snapshot.insertItems([inserted.objectID], beforeItem: snapshot.itemIdentifiers.first!) - continue - } - - var index = 0 - while index < snapshot.itemIdentifiers.count, - let pub = publishedForItem(snapshot.itemIdentifiers[index]), - insertedPublished < pub { - index += 1 - } - - // index is the index of the first item which the inserted one was published after - // (i.e., the item that should appear immediately after inserted in the list) - - if index == snapshot.itemIdentifiers.count { - snapshot.appendItems([inserted.objectID], toSection: .items) - } else { - snapshot.insertItems([inserted.objectID], beforeItem: snapshot.itemIdentifiers[index]) - } - } - - DispatchQueue.main.async { - self.dataSource.apply(snapshot, animatingDifferences: false) - } - } - } else { - DispatchQueue.main.async { - self.dataSource.apply(snapshot, animatingDifferences: false) - } - } + print("\(id) Applying snapshot") + self.dataSource.apply(snapshot, animatingDifferences: false) } }