Improve Pages List performance (#22070)

RELEASE-NOTES.txt

@@ -9,6 +9,7 @@
 * [*] [internal] Fix an issue with scheduling of posts not working on iOS 17 with Xcode 15 [#22012]
 * [*] [internal] Remove SDWebImage dependency from the app and improve cache cost calculation for GIFs [#21285]
 * [*] Stats: Fix an issue where sites for clicked URLs do not open [#22061]
+* [*] Improve pages list performance when there are hundreds of pages in the site [#22070]
 * [*] Fix an issue with local thumbnails for GIFs inserted to Site Media not being animated [#22083]
 * [*] [internal] Make Reader web views inspectable on iOS 16.4 and higher [#22077]
 * [*] [internal] Add workarounds for large emoji on P2. [#22080]

WordPress/Classes/Services/PostRepository.swift

@@ -390,7 +390,7 @@ extension PostRepository {
     ///   - statuses: Only fetch pages whose status is included in the given statues.
     ///   - blogID: Object ID of the site.
     /// - Returns: A `Task` instance representing the fetching. The fetch pages API requests will stop if the task is cancelled.
-    func fetchAllPages(statuses: [BasePost.Status], in blogID: TaggedManagedObjectID<Blog>) -> Task<[TaggedManagedObjectID<Page>], Swift.Error> {
+    func fetchAllPages(statuses: [BasePost.Status], authorUserID: NSNumber? = nil, in blogID: TaggedManagedObjectID<Blog>) -> Task<[TaggedManagedObjectID<Page>], Swift.Error> {
         Task {
             let pageSize = 100
             var allPages = [TaggedManagedObjectID<Page>]()
@@ -401,7 +401,7 @@ extension PostRepository {
                 let current = try await fetch(
                     type: Page.self,
                     statuses: statuses,
-                    authorUserID: nil,
+                    authorUserID: authorUserID,
                     range: pageRange,
                     deleteOtherLocalPosts: false,
                     in: blogID

WordPress/Classes/Utility/PageTree.swift

@@ -2,28 +2,35 @@ final class PageTree {
 
     // A node in a tree, which of course is also a tree itself.
     private class TreeNode {
-        let page: Page
+        struct PageData {
+            var postID: NSNumber?
+            var parentID: NSNumber?
+        }
+        let pageID: TaggedManagedObjectID<Page>
+        let pageData: PageData
         var children = [TreeNode]()
         var parentNode: TreeNode?
 
         init(page: Page, children: [TreeNode] = [], parentNode: TreeNode? = nil) {
-            self.page = page
+            self.pageID = TaggedManagedObjectID(page)
+            self.pageData = PageData(postID: page.postID, parentID: page.parentID)
             self.children = children
             self.parentNode = parentNode
         }
 
         // The `PageTree` type is used to loaded
         // Some page There are pages  They are pages that doesn't belong to the root level, but their parent pages haven't been loaded yet.
         var isOrphan: Bool {
-            (page.parentID?.int64Value ?? 0) > 0 && parentNode == nil
+            (pageData.parentID?.int64Value ?? 0) > 0 && parentNode == nil
         }
 
-        func dfsList() -> [Page] {
+        func dfsList(in context: NSManagedObjectContext) throws -> [Page] {
             var pages = [Page]()
-            _ = depthFirstSearch { level, node in
-                node.page.hierarchyIndex = level
-                node.page.hasVisibleParent = node.parentNode != nil
-                pages.append(node.page)
+            _ = try depthFirstSearch { level, node in
+                let page = try context.existingObject(with: node.pageID)
+                page.hierarchyIndex = level
+                page.hasVisibleParent = node.parentNode != nil
+                pages.append(page)
                 return false
             }
             return pages
@@ -35,18 +42,18 @@ final class PageTree {
         ///     a boolean value indicate whether the search should be stopped.
         /// - Returns: `true` if search has been stopped by the closure.
         @discardableResult
-        func depthFirstSearch(using closure: (Int, TreeNode) -> Bool) -> Bool {
-            depthFirstSearch(level: 0, using: closure)
+        func depthFirstSearch(using closure: (Int, TreeNode) throws -> Bool) rethrows -> Bool {
+            try depthFirstSearch(level: 0, using: closure)
         }
 
-        private func depthFirstSearch(level: Int, using closure: (Int, TreeNode) -> Bool) -> Bool {
-            let shouldStop = closure(level, self)
+        private func depthFirstSearch(level: Int, using closure: (Int, TreeNode) throws -> Bool) rethrows -> Bool {
+            let shouldStop = try closure(level, self)
             if shouldStop {
                 return true
             }
 
             for child in children {
-                let shouldStop = child.depthFirstSearch(level: level + 1, using: closure)
+                let shouldStop = try child.depthFirstSearch(level: level + 1, using: closure)
                 if shouldStop {
                     return true
                 }
@@ -77,7 +84,7 @@ final class PageTree {
             assert(parentID != 0)
 
             return depthFirstSearch { _, node in
-                if node.page.postID == parentID {
+                if node.pageData.postID == parentID {
                     node.children.append(contentsOf: newNodes)
                     newNodes.forEach { $0.parentNode = node }
                     return true
@@ -136,7 +143,7 @@ final class PageTree {
             nodes.remove(atOffsets: relocated)
             orphanNodes = nodes.enumerated().reduce(into: [:]) { indexes, node in
                 if node.element.isOrphan {
-                    let parentID = node.element.page.parentID ?? 0
+                    let parentID = node.element.pageData.parentID ?? 0
                     indexes[parentID, default: []].append(node.offset)
                 }
             }
@@ -145,7 +152,7 @@ final class PageTree {
 
     private func add(_ newNodes: [TreeNode]) {
         newNodes.forEach { newNode in
-            let parentID = newNode.page.parentID ?? 0
+            let parentID = newNode.pageData.parentID ?? 0
 
             // If the new node is at the root level, then simply add it as a child.
             if parentID == 0 {
@@ -170,14 +177,14 @@ final class PageTree {
 
     /// Move all the nodes in the given argument to the current page tree.
     private func merge(subtree: PageTree) {
-        var parentIDs = subtree.nodes.reduce(into: Set()) { $0.insert($1.page.parentID ?? 0) }
+        var parentIDs = subtree.nodes.reduce(into: Set()) { $0.insert($1.pageData.parentID ?? 0) }
         // No need to look for root level
         parentIDs.remove(0)
         // Look up parent nodes upfront, to avoid repeated iteration for each node in `subtree`.
         let parentNodes = findNodes(postIDs: parentIDs)
 
         subtree.nodes.forEach { newNode in
-            let parentID = newNode.page.parentID ?? 0
+            let parentID = newNode.pageData.parentID ?? 0
 
             // If the new node is at the root level, then simply add it as a child
             if parentID == 0 {
@@ -214,7 +221,7 @@ final class PageTree {
 
             // Using BFS under the assumption that page tree in most sites is a shallow tree, where most pages are in top layers.
             child.breadthFirstSearch { node in
-                let postID = node.page.postID ?? 0
+                let postID = node.pageData.postID ?? 0
                 let foundIndex = ids.firstIndex(of: postID)
                 if let foundIndex {
                     ids.remove(at: foundIndex)
@@ -227,15 +234,19 @@ final class PageTree {
         return result
     }
 
-    func hierarchyList() -> [Page] {
-        nodes.reduce(into: []) {
-            $0.append(contentsOf: $1.dfsList())
+    func hierarchyList(in context: NSManagedObjectContext) throws -> [Page] {
+        try nodes.reduce(into: []) {
+            try $0.append(contentsOf: $1.dfsList(in: context))
         }
     }
 
-    static func hierarchyList(of pages: [Page]) -> [Page] {
+    static func hierarchyList(of pages: [Page]) throws -> [Page] {
+        guard let context = pages.first?.managedObjectContext else {
+            return []
+        }
+
         let tree = PageTree()
         tree.add(pages)
-        return tree.hierarchyList()
+        return try tree.hierarchyList(in: context)
     }
 }

WordPress/Classes/ViewRelated/Pages/PageListViewController+Menu.swift

@@ -66,7 +66,9 @@ extension PageListViewController: InteractivePostViewDelegate {
 
     func setParent(for apost: AbstractPost) {
         guard let page = apost as? Page else { return }
-        setParentPage(for: page)
+        Task {
+            await setParentPage(for: page)
+        }
     }
 
     func setHomepage(for apost: AbstractPost) {

WordPress/Classes/ViewRelated/Pages/PageListViewController.swift

@@ -51,6 +51,8 @@ final class PageListViewController: AbstractPostListViewController, UIViewContro
 
     private var pages: [Page] = []
 
+    private var fetchAllPagesTask: Task<[TaggedManagedObjectID<Page>], Error>?
+
     // MARK: - Convenience constructors
 
     @objc class func controllerWithBlog(_ blog: Blog) -> PageListViewController {
@@ -131,6 +133,11 @@ final class PageListViewController: AbstractPostListViewController, UIViewContro
     override func viewWillDisappear(_ animated: Bool) {
         super.viewWillDisappear(animated)
         QuickStartTourGuide.shared.endCurrentTour()
+
+        if self.isMovingFromParent {
+            fetchAllPagesTask?.cancel()
+            fetchAllPagesTask = nil
+        }
     }
 
     override func traitCollectionDidChange(_ previousTraitCollection: UITraitCollection?) {
@@ -165,6 +172,22 @@ final class PageListViewController: AbstractPostListViewController, UIViewContro
         return .page
     }
 
+    @MainActor
+    override func syncPosts(isFirstPage: Bool) async throws -> SyncPostResult {
+        let coreDataStack = ContextManager.shared
+        let filter = filterSettings.currentPostListFilter()
+        let author = filterSettings.shouldShowOnlyMyPosts() ? blogUserID() : nil
+        let blogID = TaggedManagedObjectID(blog)
+
+        let repository = PostRepository(coreDataStack: coreDataStack)
+        let task = repository.fetchAllPages(statuses: filter.statuses, authorUserID: author, in: blogID)
+        self.fetchAllPagesTask = task
+
+        let posts = try await task.value.map { try coreDataStack.mainContext.existingObject(with: $0) }
+
+        return (posts, false)
+    }
+
     override func syncHelper(_ syncHelper: WPContentSyncHelper, syncContentWithUserInteraction userInteraction: Bool, success: ((Bool) -> ())?, failure: ((NSError) -> ())?) {
         // The success and failure blocks are called in the parent class `AbstractPostListViewController` by the `syncPosts` method. Since that class is
         // used by both this one and the "Posts" screen, making changes to the sync helper is tough. To get around that, we make the fetch settings call
@@ -210,17 +233,55 @@ final class PageListViewController: AbstractPostListViewController, UIViewContro
     override func updateAndPerformFetchRequest() {
         super.updateAndPerformFetchRequest()
 
-        reloadPages()
+        Task {
+            await reloadPagesAndUI()
+        }
     }
 
-    private func reloadPages() {
+    @MainActor
+    private func reloadPagesAndUI() async {
         let status = filterSettings.currentPostListFilter().filterType
         let pages = (fetchResultsController.fetchedObjects ?? []) as! [Page]
+
         if status == .published {
-            self.pages = pages.setHomePageFirst().hierarchySort()
+            let coreDataStack = ContextManager.shared
+            let pageIDs = pages.map { TaggedManagedObjectID($0) }
+
+            do {
+                self.pages = try await buildPageTree(pageIDs: pageIDs)
+                    .hierarchyList(in: coreDataStack.mainContext)
+            } catch {
+                DDLogError("Failed to reload published pages: \(error)")
+            }
         } else {
             self.pages = pages
         }
+
+        tableView.reloadData()
+        refreshResults()
+    }
+
+    /// Build page hierachy in background, which should not take long (less than 2 seconds for 6000+ pages).
+    @MainActor
+    func buildPageTree(pageIDs: [TaggedManagedObjectID<Page>]? = nil, request: NSFetchRequest<Page>? = nil) async throws -> PageTree {
+        assert(pageIDs != nil || request != nil, "`pageIDs` and `request` can not both be nil")
+
+        let coreDataStack = ContextManager.shared
+        return try await coreDataStack.performQuery { context in
+            var pages = [Page]()
+
+            if let pageIDs {
+                pages = try pageIDs.map(context.existingObject(with:))
+            } else if let request {
+                pages = try context.fetch(request)
+            }
+
+            pages = pages.setHomePageFirst()
+
+            let tree = PageTree()
+            tree.add(pages)
+            return tree
+        }
     }
 
     override func syncContentEnded(_ syncHelper: WPContentSyncHelper) {
@@ -241,9 +302,9 @@ final class PageListViewController: AbstractPostListViewController, UIViewContro
     }
 
     override func controllerDidChangeContent(_ controller: NSFetchedResultsController<NSFetchRequestResult>) {
-        reloadPages()
-        tableView.reloadData()
-        refreshResults()
+        Task {
+            await reloadPagesAndUI()
+        }
     }
 
     // MARK: - Core Data
@@ -394,16 +455,14 @@ final class PageListViewController: AbstractPostListViewController, UIViewContro
 
     // MARK: - Cell Action Handling
 
-    func setParentPage(for page: Page) {
+    @MainActor
+    func setParentPage(for page: Page) async {
         let request = NSFetchRequest<Page>(entityName: Page.entityName())
         let filter = PostListFilter.publishedFilter()
         request.predicate = filter.predicate(for: blog, author: .everyone)
         request.sortDescriptors = filter.sortDescriptors
         do {
-            var pages = try managedObjectContext()
-                .fetch(request)
-                .setHomePageFirst()
-                .hierarchySort()
+            var pages = try await buildPageTree(request: request).hierarchyList(in: ContextManager.shared.mainContext)
             if let index = pages.firstIndex(of: page) {
                 pages = pages.remove(from: index)
             }