ForEachStore - Scrolling Very Laggy When Sending Actions

[AAAstorga]

Hello,

I am working on a music app and have I have a search page. The user can search for songs and get a list of songs back. Each song row has play functionality so you can listen to the preview. When playing a preview, scrolling becomes insanely laggy (even when there are only 20 items). I am not sure why this is happening and was hoping to get some ideas.

1. I have a PreviewPlayerClient which is used in Environment.

import AVFoundation
import Combine
import ComposableArchitecture

extension PreviewPlayerClient {
  static let live: PreviewPlayerClient = { () -> PreviewPlayerClient in
    var client = PreviewPlayerClient()

    let previewPlayerHolder = PreviewPlayerHolder()

    client.subscribe = {
      previewPlayerHolder.publisher
    }

    client.playPreview = { song in
      Task.init {
        await previewPlayerHolder.playPreview(song: song)
      }
    }
    
    client.pause = {
      Task.init {
        await previewPlayerHolder.pause()
      }
    }

    return client
  }()
}

actor PreviewPlayerHolder {
  private let subscriber = PassthroughSubject<PreviewPlayerClient.Action, Never>()
  private let player = AVPlayer()
  nonisolated lazy var publisher = subscriber.eraseToEffect()
  
  private var currentUrl: URL?
  
  init() {
    observeProgress()
  }
  
  func playPreview(song: Song) {
    pause()
    
    // Resuming a pause
    if (song.previewUrl == currentUrl) {
      player.play()
      return
    }
    
    
    if player.currentItem != nil {
      subscriber.send(.didFinishPlayback)
    }
    
    guard let previewUrl = song.previewUrl else {
      return
    }
    subscriber.send(.prepareForPlayback(song))
    initializeAVAudioSession()
    
    player.replaceCurrentItem(with: .init(url: previewUrl))
    self.currentUrl = previewUrl
    player.play()
  }
  
  func pause() {
    player.pause()
  }
  
  private func initializeAVAudioSession() {
    try? AVAudioSession.sharedInstance().setCategory(.playback)
    try? AVAudioSession.sharedInstance().setActive(true)
  }
  
  private func observeProgress() {
    player.addPeriodicTimeObserver(forInterval: CMTime(value: 1, timescale: 5), queue: .main, using: { time in
      if let duration = self.player.currentItem?.duration {
        let duration = CMTimeGetSeconds(duration), time = CMTimeGetSeconds(time)
        let progress = time / duration
        if !progress.isNaN {
          self.subscriber.send(.didChangeProgress(Float(progress)))
          
          if progress >= 1.0 {
            self.subscriber.send(.didFinishPlayback)
          }
        }
      }
    })
  }
}

2. My State and Actions for the parent view

struct AddSongsState: Equatable {
   // omitted irrelevant state
  var recentSongs: IdentifiedArrayOf<SearchSongState> = []
}

enum AddSongsAction {
 // omitted other actions  
  case recentSearchSong(id: Song.ID, action: SearchSongAction)
}

struct AddSongsEnvironment {
  var mainQueue: AnySchedulerOf<DispatchQueue>
  var previewPlayerClient: PreviewPlayerClient
}

let addSongsReducer: Reducer<AddSongsState, AddSongsAction, AddSongsEnvironment> = Reducer.combine(
  searchSongReducer.forEach(state: \.recentSongs, action: /AddSongsAction.recentSearchSong(id:action:), environment: { .init(mainQueue: $0.mainQueue, previewPlayerClient: $0.previewPlayerClient )}),
  
  Reducer { state, action, environment in
    switch action {
      case .recentSearchSong:
        return .none
    }
  }
)

3. My state and action for each child

struct PreviewPlayerClientId: Hashable {
  let id: Song.Id
}

struct SearchSongState: Equatable, Identifiable {
  let id: Song.Id
  let song: Song
  
  var isPlaying = false
  var progress: Float = 1.0
  
}

enum SearchSongAction: Equatable {
  case playToggled
  case previewPlayerClient(PreviewPlayerClient.Action)
}

struct SearchSongEnvironment {
  var mainQueue: AnySchedulerOf<DispatchQueue>
  var previewPlayerClient: PreviewPlayerClient
}

let searchSongReducer = Reducer<SearchSongState, SearchSongAction, SearchSongEnvironment> { state, action, environment in
  switch action {
    case .playToggled:
      if state.isPlaying {
        state.isPlaying = false
        environment.previewPlayerClient.pause()
        return .cancel(id: PreviewPlayerClientId(id: state.song.id))
      } else {
        state.isPlaying = true
        environment.previewPlayerClient.playPreview(state.song)
        return environment.previewPlayerClient.subscribe().receive(on: environment.mainQueue).eraseToEffect().map(SearchSongAction.previewPlayerClient).cancellable(id: PreviewPlayerClientId(id: state.song.id), cancelInFlight: true)
      }      
    case .previewPlayerClient(.didFinishPlayback):
      state.isPlaying = false
      state.progress = 1.0
      return .none
    case .previewPlayerClient(.prepareForPlayback(let songToPlay)):
      state.isPlaying = state.song.id == songToPlay.id
      state.progress = state.isPlaying ? 0.0 : 1.0
      return .none
    case .previewPlayerClient(.didChangeProgress(let progress)):
      print(progress)
      if state.isPlaying {
        state.progress = progress
      }
      return .none
  }
}

4. My parent view (omitted parts that aren't necessary)

ForEachStore(self.store.scope(state: \.recentSongs, action: AddSongsAction.recentSearchSong(id:action:))) { recentSongStore in
                WithViewStore(recentSongStore) { vs in
                  SearchSongView(viewStore: vs)
                  Spacer.withHeight(16)
                }
              }

5. And finally, my child view

struct SearchSongView: View {
  
  @ObservedObject var viewStore: ViewStore<SearchSongState, SearchSongAction>

  init(viewStore: ViewStore<SearchSongState, SearchSongAction>) {
    self.viewStore = viewStore
  }

  var body: some View {
      HStack(alignment: .center, spacing: 0) {
        ZStack {
          KFImage(viewStore.song.albumCoverUrl).loadImmediately().resizable().frame(width: 60, height: 60).cornerRadius(4)

          if viewStore.song.previewUrl != nil {
            Color.black.opacity(0.5).frame(width: 60, height: 60).cornerRadius(4)
            PlayPauseWithProgressView(viewStore: viewStore) // This updates the progress bar based on viewStore.progress
          }
        }

        VStack(alignment: .leading, spacing: 2) {
          Text(viewStore.song.name).lineLimit(1).font(.custom(.avenirNextDemiBold, size: 14))
          Text(viewStore.song.artist).lineLimit(1).font(.custom(.avenirNext, size: 14))
        }.padding(.leading, 16)
      }.padding(.horizontal, 16)
    }
}

Sorry for all the code, but I am trying to have you get a complete picture. When no song is playing, the scroll perf is great. When a song is playing the PreviewPlayerClient emits an Action every 0.2 seconds with the new progress and that action seems to be lagging the view. I added some logging and each SearchSongView (as seen in 4) is recomputing when the progress updates even though really only one should be updating? How do I stop having the ForEachView recompute every single child?

Thank you all so much for any tips!

[stephencelis]

4. My parent view (omitted parts that aren't necessary)

Can you check to see how often this parent view is being re-rendered? What state is it observing, if any, via WithViewStore, etc.? If it's observing any state that updates where the ForEachStore is rendered in the hierarchy, every ForEachStore child view will recompute at this time.

[AAAstorga]

Hey @stephencelis Thanks for the note. Here is what I have.

1. UserTopSongsView (shows the top 3 songs and then a see all button for the full list)

struct UserTopSongsView: View {
  
  let store: Store<AddSongsState, AddSongsAction>
  @ObservedObject var viewStore: ViewStore<AddSongsState, AddSongsAction>
  init(store: Store<AddSongsState, AddSongsAction>) {
    self.store = store
    self.viewStore = ViewStore(store)
  }

  var body: some View {
    if !viewStore.recentSongs.isEmpty {
      HStack {
        Text("Your top songs").font(.custom(.avenirNextBold, size: 16))
        Spacer()

        if viewStore.recentSongs.count > 3 {
          NavigationLink(destination: {
            AddSongsDetailView(title: "Your top songs", selectAllAction: { }, store: self.store) {
              ForEachStore(self.store.scope(state: \.recentSongs, action: AddSongsAction.recentSearchSong(id:action:))) { recentSongStore in
                WithViewStore(recentSongStore) { vs in
                  SearchSongView(viewStore: vs)
                  Spacer.withHeight(16)
                }
              }
            }
          }) {
            Text("See All").font(.custom(.avenirNext, size: 16))
          }.accentColor(.black)
        }
      }.padding(.all, 16)
      


      ForEachStore(self.store.scope { addSongState in
        return IdentifiedArrayOf<SearchSongState>(uniqueElements: addSongState.recentSongs.prefix(3))
      } action: { id, action in
        return AddSongsAction.recentSearchSong(id: id, action: action)
      }) { (recentSongStore: Store<SearchSongState, SearchSongAction>) in
        WithViewStore(recentSongStore) { vs in
          SearchSongView(viewStore: vs)
          Spacer.withHeight(16)
        }
      }

      Divider().padding(.leading, 16)
    }
  }
}

I plan to reuse the AddSongsDetailView (as seen when pressing See all) so I made it take a () -> Content.

2. AddSongsDetailView

struct AddSongsDetailView<Content>: View where Content: View {
  @Environment(\.presentationMode) var presentation

  let title: String
  var selectAllAction: () -> Void
  let content: () -> Content

  let store: Store<AddSongsState, AddSongsAction>

  init(title: String, selectAllAction: @escaping () -> Void, store: Store<AddSongsState, AddSongsAction>, @ViewBuilder content: @escaping () -> Content) {
    self.title = title
    self.selectAllAction = selectAllAction
    self.store = store
    self.content = content
  }

  var body: some View {
    MixtapeBackgroundZStack {
      VStack(spacing: 0) {
        WithViewStore(self.store.scope(state: { State(isEmpty: $0.selectedSongs.isEmpty, count: $0.selectedSongs.count) })) { viewStore in
          HStack {
            Button(action: { presentation.wrappedValue.dismiss() }) {
              Image(systemName: "chevron.backward").resizable().aspectRatio(contentMode: .fit).frame(width: 20.0, height: 20.0)
            }.accentColor(.black)
            Spacer()
            VStack {
              Text("Add Songs").font(.custom(.avenirNextBold, size: viewStore.state.isEmpty ? 16 : 14)).frame(width: 100, alignment: .center)
              if !viewStore.state.isEmpty {
                Text("\(viewStore.state.count) Song\(viewStore.state.count == 1 ? "" : "s") added").font(.custom(.avenirNext, size: 14))
              }
            }.animation(.easeOut, value: viewStore.state.isEmpty)
            Spacer()
            Button(action: {}) {
              Image("check").resizable().aspectRatio(contentMode: .fit).frame(width: 20.0, height: 23.0)
            }.accentColor(.black)
          }.padding(.horizontal, 16).frame(height: 60).background(Color("SystemGray6"))
        }
        ScrollView(.vertical, showsIndicators: false) {
          HStack {
            Text(self.title).font(.custom(.avenirNextBold, size: 16))
            Spacer()
            Button(action: self.selectAllAction) {
              Text("Select All").font(.custom(.avenirNext, size: 16))
            }.accentColor(.black)
          }.padding(.all, 16)
          LazyVStack(spacing: 0) {
            content()
          }
        }
      }
    }.navigationBarHidden(true)
  }
}

extension AddSongsDetailView {
  struct State: Equatable {
    let isEmpty: Bool
    let count: Int
  }
}

You can see near the bottom, I call content() function. I am not sure what is triggering excess recomputes. Do you see anything?

[mbrandonw]

Hey @AAAstorga, Stephen is away right now, but I think I'm seeing the problem with the code you posted.

In UserTopSongsView you are observing all of AddSongsState when you construct the view store:

self.viewStore = ViewStore(store)

However, if you look at the body of the view you see that there are only two pieces of data you actually need from the view store:

if !viewStore.recentSongs.isEmpty

...

if viewStore.recentSongs.count > 3

This means you are re-computing this view any time a single piece of data changes in the recentSongs array, even though the view just needs essentially two booleans to do its job. So, you should introduce some view state to that view so that you can chisel away the state to just those two fields.

In fact, I see you are already doing that for AddSongsDetailView, so I would suggest trying that for UserTopSongsView too.

[AAAstorga]

Hey @mbrandonw I think this is helping, but I have a question about my whole understanding of tca.

Let's say I have the following structure.
HomeView with ForEachStore of items and clicking on an item takes you to Item Details and Item Details takes you to the Seller Profile and Seller Profile takes you to a list of the Sellers preferences. (Made up example, just trying to show deeply nested state)

So Home -> Details -> Seller Profile -> Seller Preferences.

Now our HomeState will have an IdentifiableArrayOf.

Each ItemState would have SellerProfileState and the SellerProfileState would have SellerPreferencesState.

Assuming the state is scoped and pulledback properly, would modifying the SellerPreferencesState in the SellerPreferences view cause every view down the chain to recompute?

This is the kind of behavior I'm seeing and I'm not sure I understand why.

Like would the ForEachStore on the HomeView recompute each item in the ForEach loop?

This is assuming everything is properly scoped with ViewStores and what not.

In this example (similar to my actual code), I am seeing HomeView and all other views recompute on every change in the leaf view. This is especially an issue in my use case because I am sending ViewStore actions once every 0.25s (ProgressView).

Please let me know if my question did not make sense. I'm subscribed to PointFree and have watched videos but I'm still missing this super important part of the puzzle.

Thank you so much!

[AlexisQapa]

Any view used within a WithViewStore block or having as @ObservedObject var viewStore: ViewStore<, > will be redrawn if it's viewState change. One thing to keep in mind is if a view is redrawn it redraws all of its children, so if all your hierarchy is redrawn each time a leaf changes there is a top level view which isn't properly scoped.

While you can pass a store containing child states along the hierarchy be sure to not include child states inside a viewStore.

Reading your example I think your issue is that you are using

@ObservedObject var viewStore: ViewStore<AddSongsState, AddSongsAction>

in your top level view.

[mbrandonw]

I think @AlexisQapa covered it all, but just wanted to add a little bit more info.

As stated before, when constructing a ViewStore you are observing all changes to that state, even if the state isn't being directly accessed in the view. So, in the hierarchy that you proposed, Home -> Details -> Seller Profile -> Seller Preferences, if you just construct a ViewStore observing all of home's domain, then yes, the body will re-compute for every change to state in details, profile and preferences.

It's worth noting that this is a problem in vanilla SwiftUI too. If you look at any of Apple's samples (e.g. Landmarks, Fruta, Scrum dinger, etc.) you will see the same behavior. Just open any of those projects and place a let _ = Self._printChanges() at the top of each var body and you will see that views are being re-computed for every little change. For example, in the Landmarks app, if you edit the name in the profile screen, both of the root tab views re-compute their bodies, even though they don't use that data at all. 😬

The way you solve this in vanilla SwiftUI (as far as I know) is to split the domain into many view models, but that comes with additional complexities like figuring out how view models can communicate with each other.

The way you solve this in TCA is to scope the store one additional time before creating a ViewStore. You can create a little ViewState struct that is specific to only your view, and that struct can hold the bare minimum of state that your view needs to do its job, and then you .scope the store to the ViewState before constructing the ViewStore. This makes it so that your view re-computes its body only when state changes that it actually cares about. Any changes to child state will be completely ignored.

For a concrete example, in our app isowords we use this pattern a lot. The most important example is at the very root of the application. The AppState struct holds the state for the entire application. But the AppView only needs a time part of that state. In fact, it only needs 2 booleans, and so we create a ViewState to hold that. This means the root view will re-compute its body only when one of those two booleans changes, no matter what is happening inside the child views.

For more examples of this pattern just search for "viewstate" in the isowords code base. We also have some episodes covering these topics:

• Adaptation: this is the series of episodes where we first showed potential performance problems in TCA and solved it by introducing the ViewStore concept.
• View stores and scoping: This is a more recent episode where show make some performance improvements to Store.scope's implementation, but before doing that we cover the basics of ViewStore from scratch.

Hope this helps!

[AAAstorga]

Thank you all for the responses. It makes a lot of sense and I see how we can really improve performance by slicing off small pieces of state. The one thing I am still not understanding though is why the following body is recomputing?

import ComposableArchitecture
import SwiftUI

struct UserTopSongsView: View {
  let store: Store<UserTopSongsState, UserTopSongsAction>

  init(store: Store<UserTopSongsState, UserTopSongsAction>) {
    self.store = store
  }

  var body: some View {
    let _ = Self._printChanges()

   // Scoping to just care about 1 Int value
    WithViewStore(self.store.scope(state: State.init)) { viewStore in
      VStack {
        if viewStore.recentSongCount > 0 {
          HStack {
            Text("Your top songs").font(.custom(.avenirNextBold, size: 16))
            Spacer()

            if viewStore.recentSongCount > 3 {
              NavigationLink(destination: {
                UserTopSongsDetailView(store: self.store)
              }) {
                Text("See All").font(.custom(.avenirNext, size: 16))
              }.accentColor(.black)
            }
          }.padding(.all, 16)

          ForEachStore(self.store.scope(state: \.topThreeSongs, action: UserTopSongsAction.songs(id:action:))) { searchSongStore in
            SearchSongView(store: searchSongStore)
            Spacer.withHeight(16)
          }
          Divider().padding(.leading, 16)
        }
      }.onLoad { viewStore.send(.onLoad) }
    }.debug("UserTopSongsView")
  }
}

extension UserTopSongsView {
  struct State: Equatable {
    let recentSongCount: Int

    init(state: UserTopSongsState) {
      self.recentSongCount = state.songs.count
    }
  }
}

So from my understanding the body of UserTopSongsView should only recompute when recentSongCount changes.

For more reference here is the UserTopSongsState:

struct UserTopSongsState: Equatable {
  var songs: IdentifiedArrayOf<SearchSongState> = [] {
    didSet {
      self.topThreeSongs = IdentifiedArrayOf(uniqueElements: songs.prefix(3))
    }
  }
  var topThreeSongs: IdentifiedArrayOf<SearchSongState> = []
}

In this code snippet there is UserTopSongsDetailView(store: self.store). In here, I have a ForEachStore that loops on UserTopSongsState.songs and the child in the ForEachStore might modify their own state based on actions. I am stumped to see why the code above is re-computing?

After adding the .debug(...) on the WithViewStore I see:

UserTopSongsView: @self changed.
UserTopSongsView: Evaluating WithViewStore<State, UserTopSongsAction, ...>.body

Over and over again even though recentSongCount is not changing its value.

I hope I am not missing something obvious. This is all coming together and I am starting to fully realize the power of TCA. I appreciate all the comments so far in helping me understand more.

Thanks again!

[stephencelis]

UserTopSongsView: @self changed. indicates that the entire view is being created, not merely updated, so it appears that the parent of UserTopSongsView is also potentially listening to too much state, and thus re-rendering and re-creating the UserTopSongsView of its body. Do you have more layers of view/state to peel back?

[AAAstorga]

Just to close out this thread -- I fixed the problem. I was not scoping State that well in my ViewStores and that caused so much unnecessary rendering. After fixing it, my scrolling is now buttery smooth. Thank you so much for all the help!

[mbrandonw]

That's great to hear @AAAstorga, glad it worked out! 😁