Performance issue related to CurrentValueRelay.Subscription

[takehilo]

Our app has a vertically scrolling news feed using UICollectionView and TCA (version: 1.6.0). The cells are not of a single type; different types of cells are displayed for each section. The cell views are implemented in SwiftUI, and we pass the scoped Store to the SwiftUI views.

Here is a snippet of the implementation:

final class NewsFeedView: UIViewController {
  private let store: StoreOf<NewsFeedReducer>
  private let viewStore: ViewStore<ViewState, NewsFeedReducer.Action>

  private typealias DataSourceType = UICollectionViewDiffableDataSource<NewsFeedDataSourceSectionId, NewsFeedDataSourceItemId>
  private var collectionView: UICollectionView!
  private var dataSource: DataSourceType!
  private var storeCancellables: [IndexPath: Cancellable] = [:]

  collectionView = UICollectionView(frame: .zero, collectionViewLayout: makeCompositionalLayout())
  dataSource = configureCellRegistration(to: collectionView)

  private func configureCellRegistration(to collectionView: UICollectionView) -> DataSourceType {
    DataSourceType(collectionView: collectionView) { [weak self] (collectionView, indexPath, itemId) -> UICollectionViewCell in
      guard let self else { return UICollectionViewCell() }

      let sectionState = store.withState(\.sections)[indexPath.section]

      switch sectionState {
      case .news:
        let cell = collectionView.dequeueReusableCell(
          withReuseIdentifier: "news",
          for: indexPath
        ) as! HostingCell<NewsSectionItemView>

        let cancellable = store
          .scope(
            state: \.sections[id: sectionState.id].news?,
            action: \.sections[id: sectionState.id].news
          )
          .ifLet { sectionStore in
            cell.setView(NewsView(store: sectionStore))
          }

        storeCancellables[indexPath] = cancellable

        return cell

      case movie:
        // ...
      }
    }
  }
}

During scrolling, we are sending some actions. For example, in the scrollViewDidScroll method of UICollectionViewDelegate, we send an action when half of the cell height is displayed.

To ensure that an action is sent only once per cell, the frequency of action dispatch is not overwhelming. When the feed is reloaded, the state is reset, and actions are sent again.

The problem arises when repeatedly reloading the feed and scrolling; gradually, scrolling becomes laggy. Initially displaying the feed and scrolling works smoothly.

After checking the memory graph, we noticed that the number of instances of CurrentValueRelay.Subscription keeps increasing. It starts with around 100 instances at app launch, but after repeating the feed reload and scrolling, especially when it becomes laggy, there are thousands of instances.

CurrentValueRelay.Subscription is created with ifLet or ViewStore.init. As these methods are called every time a cell is displayed, the number of instances keeps increasing.

Changing the implementation to not have the Store in the cell resolved the laggy scrolling issue. However, in our app, there are use cases where we need to have the Store in the cell to change the cell appearance based on state changes.

switch sectionState {
case .news:
  let cell = collectionView.dequeueReusableCell(
    withReuseIdentifier: "news",
    for: indexPath
  ) as! HostingCell<NewsSectionItemView>

  if let state = store.withState(\.sections[id: sectionState.id]?.news {
    cell.setView(NewsView(state: state, action: { self.store.send(.sections(.element(id: sectionState.id, action: .news($0))) }))
    return cell
  }

case movie:
  // ...
}

In my view, the continuous increase in CurrentValueRelay.Subscription instances is the likely cause, but are there other possible reasons? Additionally, do you have any ideas for a better implementation?

[mbrandonw]

Hi @takehilo, there is actually a PR open right now that fixes that from @iampatbrown. Could you point your project to that fork and verify that it fixes your issue? That would provide a good data point for us to evaluate the PR.

[takehilo]

@mbrandonw I tried the branch.

Even after refreshing and scrolling the feed repeatedly, the issue of continuous growth in CurrentValueRelay.Subscription has been eliminated. When cells are reused, the reference to the Cancellable at the same IndexPath is overwritten, so the old Cancellable is canceled at that time. When the Cancellable is canceled, the Subscription held by CurrentValueRelay is removed, resolving the problem of continuously increasing subscriptions.

However, the performance of our app has not improved significantly. Since the approach involves creating a Store for every cell, the number of Subscriptions tends to be high. Perhaps the approach itself may not be optimal.

As mentioned earlier, do you think the solution is to create a Store only for the necessary cells? In that case, it may not be possible to create a feed with many types of cells that require a Store. Is it currently challenging to create such a feed with TCA?

I haven't tried it yet, but is there a possibility that observation could be a solution?

[stephencelis]

@takehilo Thanks for giving things a try. View stores should only be necessary these days in SwiftUI (and they'll be going away completely in observation). Is there any reason you can't avoid those ViewStores in favor of using normal Stores instead? If you need synchronous state you can use store.withState and if you need to subscribe to state over time you can use a store.publisher. This will hopefully minimize the number of active subscriptions in your app. I also believe we could provide an API that eliminates the ifLets since you expect to unwrap an item there. We provide such an API internally that we use in SwiftUI. But if you want to explore such an API today:

-let cancellable = store
-  .scope(
-    state: \.sections[id: sectionState.id].news?,
-    action: \.sections[id: sectionState.id].news
-  )
-  .ifLet { sectionStore in
-    cell.setView(NewsView(store: sectionStore))
-  }
+if var news = store.withState(\.sections[id: sectionState.id].news) {
+  let sectionStore = store.scope(
+    state: { 
+      news = $0.sections[id: sectionState.id].news ?? news
+      return news
+    },
+    action: { .sections(id: sectionState.id, action: .news($0)) }
+  )
+  cell.setView(NewsView(store: sectionStore))
+}

This technically uses the old form of scoping, which is not cached, but that's necessary to work around a Swift bug with key paths and optional chaining right now (a bug we work around internally in the library right now).

[takehilo]

@stephencelis
When the state of the cell is updated, should we use Store.publisher instead of ViewStore to reflect the changes in the view? In other words, I understand that updating the cell's state triggers updating the data source snapshot and reloading the corresponding cell is a more appropriate approach.

Is it still advisable to avoid using ViewStore extensively? (Or, in the case of UIKit, should we use Store.publisher and avoid using ViewStore altogether?)

By using your method to scope Store without using ifLet, we have been able to modify our app. Thank you for providing specific code examples.

[stephencelis]

When the state of the cell is updated, should we use Store.publisher instead of ViewStore to reflect the changes in the view? In other words, I understand that updating the cell's state triggers updating the data source snapshot and reloading the corresponding cell is a more appropriate approach.

Is it still advisable to avoid using ViewStore extensively? (Or, in the case of UIKit, should we use Store.publisher and avoid using ViewStore altogether?)

Yup. You can prefer to call Store.publisher instead of ViewStore.publisher everywhere now, and should avoid using ViewStore wherever possible, and altogether in UIKit. ViewStore will even be deprecated soon.

By using your method to scope Store without using ifLet, we have been able to modify our app. Thank you for providing specific code examples.

I would just make sure that the ifLet is not needed. I'm not familiar enough with how your collection views load their data over time.