Merge pull request #18 from composed-swift/improve-composed-section-provider-performance

Author: JosephDuffy

.swiftpm/xcode/xcshareddata/xcschemes/Composed.xcscheme

@@ -26,7 +26,8 @@
       buildConfiguration = "Debug"
       selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
       selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
-      shouldUseLaunchSchemeArgsEnv = "YES">
+      shouldUseLaunchSchemeArgsEnv = "YES"
+      codeCoverageEnabled = "YES">
       <Testables>
          <TestableReference
             skipped = "NO">

Sources/Composed/Providers/ComposedSectionProvider.swift

@@ -29,20 +29,10 @@ open class ComposedSectionProvider: AggregateSectionProvider, SectionProviderUpd
 
     open weak var updateDelegate: SectionProviderUpdateDelegate?
 
-    /// Represents all of the children this provider contains
-    private var children: [Child] = []
-
     /// Returns all the sections this provider contains
-    public var sections: [Section] {
-        return children.flatMap { kind -> [Section] in
-            switch kind {
-            case let .section(section):
-                return [section]
-            case let .provider(provider):
-                return provider.sections
-            }
-        }
-    }
+    public private(set) var sections: [Section] = []
+
+    public private(set) var numberOfSections: Int = 0
 
     /// Returns all the providers this provider contains
     public var providers: [SectionProvider] {
@@ -55,14 +45,8 @@ open class ComposedSectionProvider: AggregateSectionProvider, SectionProviderUpd
         }
     }
 
-    public var numberOfSections: Int {
-        return children.reduce(into: 0, { result, kind in
-            switch kind {
-            case .section: result += 1
-            case let .provider(provider): result += provider.numberOfSections
-            }
-        })
-    }
+    /// Represents all of the children this provider contains
+    private var children: [Child] = []
 
     public init() { }
 
@@ -76,6 +60,15 @@ open class ComposedSectionProvider: AggregateSectionProvider, SectionProviderUpd
     public func sectionOffset(for provider: SectionProvider) -> Int {
         guard provider !== self else { return 0 }
 
+        // A quick test for if this is the last child is a small optimisation, mainly
+        // beneficial when the provider has just been appended.
+        switch children.last {
+        case .some(.provider(let lastProvider)) where lastProvider === provider:
+            return numberOfSections - provider.numberOfSections
+        default:
+            break
+        }
+
         var offset: Int = 0
 
         for child in children {
@@ -101,6 +94,15 @@ open class ComposedSectionProvider: AggregateSectionProvider, SectionProviderUpd
     }
 
     public func sectionOffset(for section: Section) -> Int {
+        // A quick test for if this is the last child is a small optimisation, mainly
+        // beneficial when the section has just been appended.
+        switch children.last {
+        case .some(.section(let lastSection)) where lastSection === section:
+            return numberOfSections - 1
+        default:
+            break
+        }
+
         var offset: Int = 0
 
         for child in children {
@@ -165,7 +167,9 @@ open class ComposedSectionProvider: AggregateSectionProvider, SectionProviderUpd
 
         updateDelegate?.willBeginUpdating(self)
         children.insert(.section(child), at: index)
+        numberOfSections += 1
         let sectionOffset = self.sectionOffset(for: child)
+        sections.insert(child, at: sectionOffset)
         updateDelegate?.provider(self, didInsertSections: [child], at: IndexSet(integer: sectionOffset))
         updateDelegate?.didEndUpdating(self)
     }
@@ -181,8 +185,10 @@ open class ComposedSectionProvider: AggregateSectionProvider, SectionProviderUpd
 
         updateDelegate?.willBeginUpdating(self)
         children.insert(.provider(child), at: index)
+        numberOfSections += child.sections.count
         let firstIndex = sectionOffset(for: child)
         let endIndex = firstIndex + child.sections.count
+        sections.insert(contentsOf: child.sections, at: firstIndex)
         updateDelegate?.provider(self, didInsertSections: child.sections, at: IndexSet(integersIn: firstIndex..<endIndex))
         updateDelegate?.didEndUpdating(self)
     }
@@ -227,10 +233,42 @@ open class ComposedSectionProvider: AggregateSectionProvider, SectionProviderUpd
 
         updateDelegate?.willBeginUpdating(self)
         children.remove(at: index)
+        numberOfSections -= sections.count
+        self.sections.removeSubrange(firstIndex ..< endIndex)
         updateDelegate?.provider(self, didRemoveSections: sections, at: IndexSet(integersIn: firstIndex..<endIndex))
         updateDelegate?.didEndUpdating(self)
     }
 
+    public func provider(_ provider: SectionProvider, didInsertSections sections: [Section], at indexes: IndexSet) {
+        assert(sections.count == indexes.count, "Number of indexes must equal number of sections inserted")
+
+        numberOfSections += sections.count
+
+        let sectionOffset = self.sectionOffset(for: provider)
+
+        indexes
+            .enumerated()
+            .map { element in
+                return (sections[element.offset], element.element + sectionOffset)
+            }
+            .forEach { element in
+                self.sections.insert(element.0, at: element.1)
+            }
+
+        updateDelegate?.provider(provider, didInsertSections: sections, at: indexes)
+    }
+
+    public func provider(_ provider: SectionProvider, didRemoveSections sections: [Section], at indexes: IndexSet) {
+        assert(sections.count == indexes.count, "Number of indexes must equal number of sections removed")
+
+        numberOfSections -= sections.count
+
+        let sectionOffset = self.sectionOffset(for: provider)
+        indexes.map { $0 + sectionOffset }.reversed().forEach { self.sections.remove(at: $0) }
+
+        updateDelegate?.provider(provider, didRemoveSections: sections, at: indexes)
+    }
+
 }
 
 // MARK:- Convenience Functions

Tests/ComposedTests/ComposedSectionProvider.swift

@@ -8,36 +8,54 @@ final class ComposedSectionProvider_Spec: QuickSpec {
 
     override func spec() {
         describe("ComposedSectionProvider") {
-            let global = ComposedSectionProvider()
-
-            let child1 = ComposedSectionProvider()
-                let child1a = ArraySection<String>()
-                let child1b = ArraySection<String>()
-            let child2 = ComposedSectionProvider()
-                let child2a = ComposedSectionProvider()
-                    let child2b = ArraySection<String>()
-                    let child2c = ArraySection<String>()
-                let child2z = ComposedSectionProvider()
-                let child2d = ArraySection<String>()
-                let child2e = ComposedSectionProvider()
-                    let child2f = ArraySection<String>()
-
-            child1.append(child1a)
-            child1.insert(child1b, after: child1a)
-
-            child2.append(child2a)
-            child2a.append(child2c)
-            child2a.insert(child2b, before: child2c)
-
-            child2.insert(child2z, after: child2a)
-            child2.append(child2d)
-            child2e.append(child2f)
-            child2.append(child2e)
-            global.append(child1)
-            global.append(child2)
-
-            it("should contain 2 global sections") {
-                expect(global.numberOfSections) == 6
+            var global: ComposedSectionProvider!
+            var child1: ComposedSectionProvider!
+                var child1a: ArraySection<String>!
+                var child1b: ArraySection<String>!
+            var child2: ComposedSectionProvider!
+                var child2a: ComposedSectionProvider!
+                    var child2b: ArraySection<String>!
+                    var child2c: ArraySection<String>!
+                    var child2d: ArraySection<String>!
+                var child2e: ComposedSectionProvider!
+                var child2f: ArraySection<String>!
+                var child2g: ComposedSectionProvider!
+                    var child2h: ArraySection<String>!
+
+            beforeEach {
+                global = ComposedSectionProvider()
+
+                child1 = ComposedSectionProvider()
+                    child1a = ArraySection<String>()
+                    child1b = ArraySection<String>()
+                child2 = ComposedSectionProvider()
+                    child2a = ComposedSectionProvider()
+                        child2b = ArraySection<String>()
+                        child2c = ArraySection<String>()
+                        child2d = ArraySection<String>()
+                    child2e = ComposedSectionProvider()
+                    child2f = ArraySection<String>()
+                    child2g = ComposedSectionProvider()
+                        child2h = ArraySection<String>()
+
+                child1.append(child1a)
+                child1.insert(child1b, after: child1a)
+
+                child2.append(child2a)
+                child2a.append(child2c)
+                child2a.insert(child2b, before: child2c)
+                child2a.insert(child2d, after: child2c)
+
+                child2.insert(child2e, after: child2a)
+                child2.append(child2f)
+                child2g.append(child2h)
+                child2.append(child2g)
+                global.append(child1)
+                global.append(child2)
+            }
+
+            it("should contain 7 global sections") {
+                expect(global.numberOfSections) == 7
             }
 
             it("cache should contain 2 providers") {
@@ -46,25 +64,37 @@ final class ComposedSectionProvider_Spec: QuickSpec {
 
             it("should return the right offsets") {
                 expect(global.sectionOffset(for: child1)) == 0
+                expect(global.sectionOffset(for: child1a)) == 0
+                expect(global.sectionOffset(for: child1b)) == 1
                 expect(global.sectionOffset(for: child2)) == 2
                 expect(global.sectionOffset(for: child2a)) == 2
-                expect(global.sectionOffset(for: child2z)) == 4
+                expect(global.sectionOffset(for: child2b)) == 2
+                expect(global.sectionOffset(for: child2c)) == 3
+                expect(global.sectionOffset(for: child2d)) == 4
                 expect(global.sectionOffset(for: child2e)) == 5
+                expect(global.sectionOffset(for: child2f)) == 5
+                expect(global.sectionOffset(for: child2g)) == 6
+                expect(global.sectionOffset(for: child2h)) == 6
 
                 expect(child2.sectionOffset(for: child2a)) == 0
-                expect(child2.sectionOffset(for: child2z)) == 2
                 expect(child2.sectionOffset(for: child2e)) == 3
+                expect(child2.sectionOffset(for: child2g)) == 4
+
+                expect(child2a.sectionOffset(for: child2b)) == 0
+                expect(child2a.sectionOffset(for: child2c)) == 1
+                expect(child2a.sectionOffset(for: child2d)) == 2
             }
 
             context("when a section is inserted after a section provider with multiple sections") {
                 var mockDelegate: MockSectionProviderUpdateDelegate!
                 var countBefore: Int!
+                var newSection: ArraySection<String>!
 
                 beforeEach {
                     mockDelegate = MockSectionProviderUpdateDelegate()
                     global.updateDelegate = mockDelegate
 
-                    let newSection = ArraySection<String>()
+                    newSection = ArraySection<String>()
                     countBefore = global.numberOfSections
 
                     global.append(newSection)
@@ -73,19 +103,38 @@ final class ComposedSectionProvider_Spec: QuickSpec {
                 it("should pass the correct indexes to the delegate") {
                     expect(mockDelegate.didInsertSectionsCalls.last!.2) == IndexSet(integer: countBefore)
                 }
+
+                it("should update the sections count") {
+                    expect(global.numberOfSections) == 8
+                }
+
+                it("should contain the correct sections") {
+                    expect(global.sections[0]) === child1a
+                    expect(global.sections[1]) === child1b
+                    expect(global.sections[2]) === child2b
+                    expect(global.sections[3]) === child2c
+                    expect(global.sections[4]) === child2d
+                    expect(global.sections[5]) === child2f
+                    expect(global.sections[6]) === child2h
+                    expect(global.sections[7]) === newSection
+                }
             }
 
             context("when a section provider is inserted after a section provider with multiple sections") {
                 var mockDelegate: MockSectionProviderUpdateDelegate!
                 var countBefore: Int!
                 var sectionProvider: ComposedSectionProvider!
+                var newSection1: ArraySection<String>!
+                var newSection2: ArraySection<String>!
 
                 beforeEach {
                     mockDelegate = MockSectionProviderUpdateDelegate()
                     global.updateDelegate = mockDelegate
                     sectionProvider = ComposedSectionProvider()
-                    sectionProvider.append(ArraySection<String>())
-                    sectionProvider.append(ArraySection<String>())
+                    newSection1 = ArraySection<String>()
+                    newSection2 = ArraySection<String>()
+                    sectionProvider.append(newSection1)
+                    sectionProvider.append(newSection2)
 
                     countBefore = global.numberOfSections
 
@@ -95,6 +144,22 @@ final class ComposedSectionProvider_Spec: QuickSpec {
                 it("should pass the correct indexes to the delegate") {
                     expect(mockDelegate.didInsertSectionsCalls.last!.2) == IndexSet(integersIn: countBefore..<(countBefore + sectionProvider.numberOfSections))
                 }
+
+                it("should update the sections count") {
+                    expect(global.numberOfSections) == 9
+                }
+
+                it("should contain the correct sections") {
+                    expect(global.sections[0]) === child1a
+                    expect(global.sections[1]) === child1b
+                    expect(global.sections[2]) === child2b
+                    expect(global.sections[3]) === child2c
+                    expect(global.sections[4]) === child2d
+                    expect(global.sections[5]) === child2f
+                    expect(global.sections[6]) === child2h
+                    expect(global.sections[7]) === newSection1
+                    expect(global.sections[8]) === newSection2
+                }
             }
 
             context("when a section located after a section provider with multiple sections is removed") {
@@ -116,6 +181,49 @@ final class ComposedSectionProvider_Spec: QuickSpec {
                 it("should pass the correct indexes to the delegate") {
                     expect(mockDelegate.didRemoveSectionsCalls.last!.2) == IndexSet(integer: countBefore - 1)
                 }
+
+                it("should contain the correct sections") {
+                    expect(global.sections[0]) === child1a
+                    expect(global.sections[1]) === child1b
+                    expect(global.sections[2]) === child2b
+                    expect(global.sections[3]) === child2c
+                    expect(global.sections[4]) === child2d
+                    expect(global.sections[5]) === child2f
+                    expect(global.sections[6]) === child2h
+                }
+            }
+
+            context("when multiple sections are removed") {
+                var mockDelegate: MockSectionProviderUpdateDelegate!
+                var countBefore: Int!
+
+                beforeEach {
+                    mockDelegate = MockSectionProviderUpdateDelegate()
+                    global.updateDelegate = mockDelegate
+
+                    countBefore = global.numberOfSections
+
+                    child2.remove(child2a)
+                }
+
+                it("should pass through the removed indexes to the delegate") {
+                    expect(mockDelegate.didRemoveSectionsCalls.last!.2) == IndexSet([0, 1, 2])
+                }
+
+                it("should update the number of sections") {
+                    expect(global.numberOfSections) == countBefore - 3
+                }
+
+                it("should pass itself to the delegate") {
+                    expect(mockDelegate.didRemoveSectionsCalls.last!.0) === child2
+                }
+
+                it("should contain the correct sections") {
+                    expect(global.sections[0]) === child1a
+                    expect(global.sections[1]) === child1b
+                    expect(global.sections[2]) === child2f
+                    expect(global.sections[3]) === child2h
+                }
             }
         }
     }