Bk/performance improvements (#272)

* Prevent DateFormatters from being initialized with CalendarViewContent * Optimize layoutSubviews to prevent unnecessary work * Lint
airbnb · Sep 22, 2023 · 343cd82 · 343cd82
1 parent 39e0b3f
commit 343cd82
Show file tree

Hide file tree

Showing 4 changed files with 64 additions and 45 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -33,6 +33,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Changed `ItemView` to determine user interaction capabilities from its content view's `hitTest` / `pointInside` functions
 - Updated content-change animations so that the same scroll offset is maintained throughout the animation
 - Changed the Swift version needed to use HorizonCalendar to 5.8
+- Optimized `layoutSubviews` to avoid doing unnecessary work updating views in some cases
 
 ## [v1.16.0](https://github.com/airbnb/HorizonCalendar/compare/v1.15.0...v1.16.0) - 2023-01-30
 

diff --git a/Sources/Internal/VisibleItemsProvider.swift b/Sources/Internal/VisibleItemsProvider.swift
@@ -70,7 +70,10 @@ final class VisibleItemsProvider {
     -> LayoutItem
   {
     let layoutItem = LayoutItem(itemType: .monthHeader(month), frame: .zero)
-    var context = VisibleItemsContext(centermostLayoutItem: layoutItem, firstLayoutItem: layoutItem)
+    var context = VisibleItemsContext(
+      centermostLayoutItem: layoutItem,
+      firstLayoutItem: layoutItem,
+      lastLayoutItem: layoutItem)
     let monthHeaderHeight = monthHeaderHeight(for: month, context: &context)
 
     let monthOrigin: CGPoint
@@ -112,7 +115,10 @@ final class VisibleItemsProvider {
     -> LayoutItem
   {
     let layoutItem = LayoutItem(itemType: .day(day), frame: .zero)
-    var context = VisibleItemsContext(centermostLayoutItem: layoutItem, firstLayoutItem: layoutItem)
+    var context = VisibleItemsContext(
+      centermostLayoutItem: layoutItem,
+      firstLayoutItem: layoutItem,
+      lastLayoutItem: layoutItem)
     let month = day.month
     let monthHeaderHeight = monthHeaderHeight(for: month, context: &context)
 
@@ -164,6 +170,7 @@ final class VisibleItemsProvider {
     var context = VisibleItemsContext(
       centermostLayoutItem: previouslyVisibleLayoutItem,
       firstLayoutItem: previouslyVisibleLayoutItem,
+      lastLayoutItem: previouslyVisibleLayoutItem,
       calendarItemModelCache: .init(
         minimumCapacity: previousCalendarItemModelCache?.capacity ?? 100))
 
@@ -283,6 +290,7 @@ final class VisibleItemsProvider {
       visibleItems: context.visibleItems,
       centermostLayoutItem: context.centermostLayoutItem,
       firstLayoutItem: context.firstLayoutItem,
+      lastLayoutItem: context.lastLayoutItem,
       visibleDayRange: visibleDayRange,
       visibleMonthRange: visibleMonthRange,
       framesForVisibleMonths: context.framesForVisibleMonths,
@@ -340,7 +348,8 @@ final class VisibleItemsProvider {
     var lastHandledLayoutItemEnumeratingForwards = previouslyVisibleLayoutItem
     var context = VisibleItemsContext(
       centermostLayoutItem: previouslyVisibleLayoutItem,
-      firstLayoutItem: previouslyVisibleLayoutItem)
+      firstLayoutItem: previouslyVisibleLayoutItem,
+      lastLayoutItem: previouslyVisibleLayoutItem)
 
     layoutItemTypeEnumerator.enumerateItemTypes(
       startingAt: previouslyVisibleLayoutItem.itemType,
@@ -417,6 +426,22 @@ final class VisibleItemsProvider {
     return itemOrigin < otherItemOrigin ? item : otherItem
   }
 
+  // Returns the layout item closest to the bottom/trailing edge of `bounds`.
+  private func lastLayoutItem(comparing item: LayoutItem, to otherItem: LayoutItem) -> LayoutItem {
+    let itemOrigin: CGFloat
+    let otherItemOrigin: CGFloat
+    switch content.monthsLayout {
+    case .vertical:
+      itemOrigin = item.frame.maxY
+      otherItemOrigin = otherItem.frame.maxY
+    case .horizontal:
+      itemOrigin = item.frame.maxX
+      otherItemOrigin = otherItem.frame.maxX
+    }
+
+    return itemOrigin > otherItemOrigin ? item : otherItem
+  }
+
   private func boundsForAnimatedUpdatePass(atOffset offset: CGPoint) -> CGRect {
     // Use a larger bounds (3x the viewport size) if we're in an animated update pass, reducing the
     // likelihood of an item popping in / out.
@@ -866,6 +891,9 @@ final class VisibleItemsProvider {
         context.firstLayoutItem = firstLayoutItem(
           comparing: layoutItem,
           to: context.firstLayoutItem)
+        context.lastLayoutItem = lastLayoutItem(
+          comparing: layoutItem,
+          to: context.lastLayoutItem)
       }
     } else {
       shouldStop = true
@@ -1234,6 +1262,7 @@ final class VisibleItemsProvider {
 private struct VisibleItemsContext {
   var centermostLayoutItem: LayoutItem
   var firstLayoutItem: LayoutItem
+  var lastLayoutItem: LayoutItem
   var firstVisibleDay: Day?
   var lastVisibleDay: Day?
   var firstVisibleMonth: Month?
@@ -1258,6 +1287,7 @@ struct VisibleItemsDetails {
   let visibleItems: Set<VisibleItem>
   let centermostLayoutItem: LayoutItem
   let firstLayoutItem: LayoutItem?
+  let lastLayoutItem: LayoutItem?
   let visibleDayRange: DayRange?
   let visibleMonthRange: MonthRange?
   let framesForVisibleMonths: [Month: CGRect]
@@ -1271,6 +1301,11 @@ struct VisibleItemsDetails {
     maxMonthHeight + heightOfPinnedContent
   }
 
+  var layoutRegion: ClosedRange<LayoutItem.ItemType>? {
+    guard let firstLayoutItem, let lastLayoutItem else { return nil }
+    return firstLayoutItem.itemType...lastLayoutItem.itemType
+  }
+
 }
 
 // MARK: - _DayRangeLayoutContext

diff --git a/Sources/Public/CalendarView.swift b/Sources/Public/CalendarView.swift
@@ -258,6 +258,10 @@ public final class CalendarView: UIView {
       invalidateIntrinsicContentSize()
     }
 
+    // Clear this so that we don't return early in our next layout pass even though our layout
+    // region might not have changed.
+    layoutRegion = nil
+
     self.content = content
     setNeedsLayout()
 
@@ -496,6 +500,7 @@ public final class CalendarView: UIView {
   private var anchorLayoutItem: LayoutItem?
   private var _visibleItemsProvider: VisibleItemsProvider?
   private var visibleItemsDetails: VisibleItemsDetails?
+  private var layoutRegion: ClosedRange<LayoutItem.ItemType>?
   private var visibleViewsForVisibleItems = [VisibleItem: ItemView]()
 
   private var isAnimatedUpdatePass = false
@@ -734,11 +739,14 @@ public final class CalendarView: UIView {
       isAnimatedUpdatePass: isAnimatedUpdatePass)
     self.anchorLayoutItem = currentVisibleItemsDetails.centermostLayoutItem
 
-    updateVisibleViews(
-      withVisibleItems: currentVisibleItemsDetails.visibleItems,
-      previouslyVisibleItems: visibleItemsDetails?.visibleItems ?? [])
+    // If our first / last layout item hasn't changed, then we haven't scrolled enough to trigger
+    // an update of visible views. This short-circuit greatly improves scroll performance.
+    if currentVisibleItemsDetails.layoutRegion != layoutRegion {
+      updateVisibleViews(withVisibleItems: currentVisibleItemsDetails.visibleItems)
+    }
 
     visibleItemsDetails = currentVisibleItemsDetails
+    layoutRegion = currentVisibleItemsDetails.layoutRegion
 
     let minimumScrollOffset = visibleItemsDetails?.contentStartBoundary.map {
       ($0 - firstLayoutMarginValue).alignedToPixel(forScreenWithScale: scale)
@@ -760,10 +768,7 @@ public final class CalendarView: UIView {
     }
   }
 
-  private func updateVisibleViews(
-    withVisibleItems visibleItems: Set<VisibleItem>,
-    previouslyVisibleItems _: Set<VisibleItem>)
-  {
+  private func updateVisibleViews(withVisibleItems visibleItems: Set<VisibleItem>) {
     var viewsToHideForVisibleItems = visibleViewsForVisibleItems
     visibleViewsForVisibleItems.removeAll(keepingCapacity: true)
 
@@ -1158,10 +1163,7 @@ extension CalendarView {
       guard cachedAccessibilityElements == nil else {
         return cachedAccessibilityElements
       }
-      guard
-        let visibleItemsDetails,
-        let visibleMonthRange
-      else {
+      guard let visibleItemsDetails, let visibleMonthRange else {
         return nil
       }
 

diff --git a/Sources/Public/CalendarViewContent.swift b/Sources/Public/CalendarViewContent.swift
@@ -188,12 +188,7 @@ public final class CalendarViewContent {
     -> CalendarViewContent
   {
     self.monthHeaderItemProvider = { [defaultMonthHeaderItemProvider] month in
-      guard let itemModel = monthHeaderItemProvider(month) else {
-        // If the caller returned nil, fall back to the default item provider
-        return defaultMonthHeaderItemProvider(month)
-      }
-
-      return itemModel
+      monthHeaderItemProvider(month) ?? defaultMonthHeaderItemProvider(month)
     }
 
     return self
@@ -223,12 +218,7 @@ public final class CalendarViewContent {
     -> CalendarViewContent
   {
     self.dayOfWeekItemProvider = { [defaultDayOfWeekItemProvider] month, weekdayIndex in
-      guard let itemModel = dayOfWeekItemProvider(month, weekdayIndex) else {
-        // If the caller returned nil, fall back to the default item provider
-        return defaultDayOfWeekItemProvider(month, weekdayIndex)
-      }
-
-      return itemModel
+      dayOfWeekItemProvider(month, weekdayIndex) ?? defaultDayOfWeekItemProvider(month, weekdayIndex)
     }
 
     return self
@@ -254,12 +244,7 @@ public final class CalendarViewContent {
     -> CalendarViewContent
   {
     self.dayItemProvider = { [defaultDayItemProvider] day in
-      guard let itemModel = dayItemProvider(day) else {
-        // If the caller returned nil, fall back to the default item provider
-        return defaultDayItemProvider(day)
-      }
-
-      return itemModel
+      dayItemProvider(day) ?? defaultDayItemProvider(day)
     }
 
     return self
@@ -407,10 +392,7 @@ public final class CalendarViewContent {
 
   /// The default `monthHeaderItemProvider` if no provider has been configured,
   /// or if the existing provider returns nil.
-  private lazy var defaultMonthHeaderItemProvider: (Month) -> AnyCalendarItemModel = { [
-    calendar,
-    monthHeaderDateFormatter
-  ] month in
+  private lazy var defaultMonthHeaderItemProvider: (Month) -> AnyCalendarItemModel = { [unowned self] month in
     let firstDateInMonth = calendar.firstDate(of: month)
     let monthText = monthHeaderDateFormatter.string(from: firstDateInMonth)
     let itemModel = MonthHeaderView.calendarItemModel(
@@ -421,18 +403,17 @@ public final class CalendarViewContent {
 
   /// The default `dayHeaderItemProvider` if no provider has been configured,
   /// or if the existing provider returns nil.
-  private lazy var defaultDayOfWeekItemProvider: (Month?, Int)
-    -> AnyCalendarItemModel = { [monthHeaderDateFormatter] _, weekdayIndex in
-      let dayOfWeekText = monthHeaderDateFormatter.veryShortStandaloneWeekdaySymbols[weekdayIndex]
-      let itemModel = DayOfWeekView.calendarItemModel(
-        invariantViewProperties: .base,
-        content: .init(dayOfWeekText: dayOfWeekText, accessibilityLabel: dayOfWeekText))
-      return itemModel
-    }
+  private lazy var defaultDayOfWeekItemProvider: (Month?, Int) -> AnyCalendarItemModel = { [unowned self] _, weekdayIndex in
+    let dayOfWeekText = monthHeaderDateFormatter.veryShortStandaloneWeekdaySymbols[weekdayIndex]
+    let itemModel = DayOfWeekView.calendarItemModel(
+      invariantViewProperties: .base,
+      content: .init(dayOfWeekText: dayOfWeekText, accessibilityLabel: dayOfWeekText))
+    return itemModel
+  }
 
   /// The default `dayItemProvider` if no provider has been configured,
   /// or if the existing provider returns nil.
-  private lazy var defaultDayItemProvider: (Day) -> AnyCalendarItemModel = { [calendar, dayDateFormatter] day in
+  private lazy var defaultDayItemProvider: (Day) -> AnyCalendarItemModel = { [unowned self] day in
     let date = calendar.startDate(of: day)
     let itemModel = DayView.calendarItemModel(
       invariantViewProperties: .baseNonInteractive,