Re-rendering sibling items when modifying a child state

[josephktcheung]

Hi,

I'm a new learner of TCA and SwiftUI, and I'm looking into the performance of ForEachStore with the Todos case study. I added .debug() to WithViewStore in TodoView like this:

    WithViewStore(self.store) { viewStore in
      HStack {
        ...
      }
      .foregroundColor(viewStore.isComplete ? .gray : nil)
    }
    .debug("TodoView")

Run the case study in simulator and add a bunch of todos. Whenever I add a new todo / edit a todo / filter todos, it triggers other sibling TodoViews to re-render (calling WithViewStore again) while state of those siblings aren't changed at all.

I believe it's because adding / editing a Todo invalidates todos state that makes the list and its items to re-render. I then start exploring ways to reduce needless rendering of unchanged siblings.

What I found is that if I make TodoView equatable then the sibling TodoViews won't re-render anymore (without the need of wrapping a EquatableView over the TodoView).

struct TodoView: View, Equatable {
  let store: Store<Todo, TodoAction>

  var body: some View {
    WithViewStore(self.store) { viewStore in
      HStack {
        Button(action: { viewStore.send(.checkBoxToggled) }) {
          Image(systemName: viewStore.isComplete ? "checkmark.square" : "square")
        }
        .buttonStyle(PlainButtonStyle())

        TextField(
          "Untitled Todo",
          text: viewStore.binding(get: \.description, send: TodoAction.textFieldChanged)
        )
      }
      .foregroundColor(viewStore.isComplete ? .gray : nil)
    }
    .debug("TodoView")
  }

    static func == (lhs: TodoView, rhs: TodoView) -> Bool {
        return ViewStore(lhs.store).state == ViewStore(rhs.store).state
    }
}

Screen.Recording.2021-09-24.at.6.53.15.PM.mp4

I'd like to know if this is the appropriate approach to solve this problem.

[iampatbrown]

Interesting... These re-renders could probably be avoided by making Store and ViewStore conform to Equatable...
@stephencelis @mbrandonw are there unwanted consequences of something like this?

// Store.swift
extension Store: Equatable where State: Equatable {
  public static func == (lhs: Store, rhs: Store) -> Bool {
    lhs.state.value == rhs.state.value
  }
}

// ViewStore.swift
extension ViewStore: Equatable where State: Equatable {
  public static func == (lhs: ViewStore, rhs: ViewStore) -> Bool {
    lhs._state.value == rhs._state.value
  }
}

With ViewStore you could probably use isDuplicate instead though.

@josephktcheung I'm not too sure about the difference between conforming a View to Equatable compared to its properties. This might be equivalent to your Todos example as a stand alone extension:

extension Store: Equatable where State: Equatable {
  public static func == (lhs: Store, rhs: Store) -> Bool {
    ViewStore(lhs).state == ViewStore(rhs).state
  }
}

[josephktcheung]

Hi @iampatbrown,

Thanks for the response! I tried adding the extension to Todos project and I tracked how many times the equality check is called by modifying the Todo.swift:

struct Todo: Equatable, Identifiable {
  var description = ""
  let id: UUID
  var isComplete = false
    
    // modified the initialiser to show new todo with shortened UUID instead of blank title
    init(id: UUID) {
        self.id = id
        self.description = "\(id.uuidString.prefix(8))"
    }
    
    init(description: String, id: UUID, isComplete: Bool) {
        self.id = id
        self.description = description
        self.isComplete = isComplete
    }
}
struct TodoView: View, Equatable {
    let store: Store<Todo, TodoAction>
    
    var body: some View {
        WithViewStore(self.store) { viewStore in
            HStack {
                Button(action: { viewStore.send(.checkBoxToggled) }) {
                    Image(systemName: viewStore.isComplete ? "checkmark.square" : "square")
                }
                .buttonStyle(PlainButtonStyle())
                
                TextField(
                    "Untitled Todo",
                    text: viewStore.binding(get: \.description, send: TodoAction.textFieldChanged)
                )
            }
            .foregroundColor(viewStore.isComplete ? .gray : nil)
        }
        .debug("TodoView")
    }
}

var countsByState: [String: Int] = [:]

extension Store: Equatable where State: Equatable {
    public static func == (lhs: Store, rhs: Store) -> Bool {
        let stateString = String(describing: type(of: ViewStore(lhs).state).self)
        countsByState[stateString] = (countsByState[stateString] ?? 0) + 1
        print("comparing store \(stateString) #\(countsByState[stateString]!)")
        return ViewStore(lhs).state == ViewStore(rhs).state
    }
}

Then whenever the equality check is called it will print something like this:

comparing store Todo #12
comparing store AppState #99

And I modified SceneDelegate to show some mock todos when app starts

class SceneDelegate: UIResponder, UIWindowSceneDelegate {
  var window: UIWindow?

  func scene(
    _ scene: UIScene,
    willConnectTo session: UISceneSession,
    options connectionOptions: UIScene.ConnectionOptions
  ) {
    self.window = (scene as? UIWindowScene).map(UIWindow.init(windowScene:))

    let rootView = AppView(
      store: Store(
        initialState: AppState(todos: .mock),
        reducer: appReducer,
        environment: AppEnvironment(
          mainQueue: .main,
          uuid: UUID.init
        )
      )
    )

    self.window?.rootViewController = UIHostingController(rootView: rootView)
    self.window?.makeKeyAndVisible()
  }
}

And by adding the extension it does stop siblings from re-rendering. However the equality check runs a ton of times when I switch the filter.
Here's the screen recording of the behaviour:

Screen.Recording.2021-09-25.at.11.32.32.AM.mp4

I can't explain why AppState's equality check is run many times (in the recording it ran 53 times for the last filter switch).

I wonder if there's a more performant solution so that the equality check can be run less, or it's something controlled by SwiftUI which is out of our control.

[iampatbrown]

Thanks for providing your code changes :) I'll have a bit of a look now

[iampatbrown]

@josephktcheung I think you've found a potential reason for not conforming Store to Equatable :) I guess it would come down to which is more expensive. Recalculating the body or performing the equality check. I'm going to keep looking into this and maybe figure out a way to benchmark it.

[josephktcheung]

@iampatbrown thanks for the quick response and look forward to the benchmark result!

[iampatbrown]

The equatability conformance definitely breaks things, most noticeably inside ScrollView/GeometryReader. Using EquatableView seems like a good choice, it would probably be better if we didn't have to wrap the stores in ViewStore to compare state though. I'll experiment with this a little more. The SwiftUI profiler can be pretty helpful for this sort of stuff too. It's worth checking out if you haven't already.

[tgrapperon]

As I understand it, ForEachStore will vend a new Store instance each time your list's body is evaluated. This effectively changes the dependencies of the TodoView's (the store instance), requiring them to produce a new body in their turn. You can look at WWDC's Demystify SwiftUI to get a better explanation about View's invalidation.

I'm not sure about what happens on SwiftUI's side with TodoView being Equatable without using EquatableView. I don't know if such behavior is documented, but I would definitely wrap them in EquatableView to avoid unexpected side-effects down the line. I don't think it's problematic in this example though, but this can definitely be useful when the body is quite complex to evaluate compared to equating the state.

[tgrapperon]

@iampatbrown Indeed, not reusing Store and ViewStore is a small inconvenience with TCA + SwiftUI right now. It doesn't have a huge impact on performance though, as SwiftUI is smart enough to not invalidate blindly whole subview trees.

One concern I have with Equatable fixing the issue when being used as it, is that its not documented. It looks like an "Happy Little Accident", and the SwiftUI way to declare you're relying on the Equatable status of a view to trigger updates is to wrap them in some EquatableView. I'm not sure for example that Environment updates are well taken into account/propagated. I'm surprised it works to begin with. So, if we rely on the equatability of the state, we should definitely opt-in and wrap TodoView inside EquatableView.

Furthermore, using equatability may work in this case, but if TodoView has a subview that depends on a scoped store, it may still refer to the original scoped instance if TodoView is hiding its store's updates behind Equatable (as the body will not be evaluated again, and the new store instance, not scoped and passed to the subview). In other words, the store instance of TodoView will be different at some point from the instance that was scoped and passed to the subview. Not an issue with the current implementation of TCA and TodoView, but I find it a little sketchy and I can't see where this can be desirable and not bite you at some point.

In my opinion, I'm not sure there's a real issue here. Evaluating body is not synonym of some work being effectively realized on the UI side, and it may be a necessary step to assess that nothing changes down the line. The issue is that TodoView is receiving a new store instance each time, and I don't know if mitigating this on the View's side is the best approach (or we should do this for every scoped store, as you have noticed).

[iampatbrown]

You make some solid points. I'm keen to explore it though. At least to know when it's worthwhile and isn't.

[iampatbrown]

@tgrapperon your intuition was right. Things really start to break down inside ScrollView/GeometryReader etc.

[stephencelis]

The ScrollView and GeometryReader behavior definitely feels like a bug in SwiftUI, so please do file feedback if you find the time. We've filed some for both of these views, but alas they're still not fixed.

It'd be nice to avoid this extra work in TCA, but as you found it looks like we can't do so by simply making ViewStore and/or WithViewStore equatable. We can't even add a more targeted equatable view wrapper around the ForEachStore's element content, since I think any geometry reader or scroll view inside it would break.

If it were possible for scoped stores to have a persistent identity, maybe we could fix the problem in the library? One idea would be that maybe WithViewStore could use @StateObject instead of @ObservedObject, thus keeping the same object around for much longer. Not sure that would work or solve the problem though 😅

[iampatbrown]

@stephencelis I was thinking the same thing about the wrapper. Interesting idea regarding the a persistent identity... it's worth exploring.

I've been under the impression we don't use @StateObject at the moment because of compatibility, is that right?

Is there an acceptable way to expose Store's state for EquatableView?

[stephencelis]

@josephktcheung Thanks for the discussion here! That's definitely a bummer, though ideally it doesn't noticeably impact most apps, and it's good to see that you have a solution for it. Hopefully we can find a fix at the library level, though!

[josephktcheung]

Hi @stephencelis,

Thanks for the reply! I hope that the library can solve this problem by itself, however it could be something beyond the library's scope. If that's the case then it'd be best if we can have something like a recipe for solving this problem in app level instead.

I used React and Redux in my web projects and they too faced the same issue: re-rendering sibling items, and redux documentation provides a solution / suggestion to it: normalising data and make the item component depend only on a single item state, the end result is that changing a single item won't cause siblings to re-render

We have something very similar to redux's solution: IdentifiedArray which extracts ids and entities from an array of items, and store scoping is kinda similar to redux's useSelector, that makes me wonder if we can learn something from redux and provide tools for users to solve this problem.

I don't know how much we can translate redux's solution to TCA, but I think it's worth looking into it from a different perspective 😊

[josephktcheung]

Hi @stephencelis, @iampatbrown,

I continue to look into the performance issue of ForEachStore and the performance gets a big hit when I start adding animation in a view that contains a ForEachStore.

I modified the 01-GettingStarted-Animations case study to replace the read me text with a simple todo list with 10,000 todos, when I move the cursor in the simulator, the frame rate is terrible.

Here's the sample code:

import Combine
import ComposableArchitecture
import SwiftUI

private let readMe = """
  This screen demonstrates how changes to application state can drive animations. Because the \
  `Store` processes actions sent to it synchronously you can typically perform animations \
  in the Composable Architecture just as you would in regular SwiftUI.
  
  To animate the changes made to state when an action is sent to the store you can pass along an \
  explicit animation, as well, or you can call `viewStore.send` in a `withAnimation` block.
  
  To animate changes made to state through a binding, use the `.animation` method on `Binding`.
  
  To animate asynchronous changes made to state via effects, use the `.animation` method provided \
  by the CombineSchedulers library to receive asynchronous actions in an animated fashion.
  
  Try it out by tapping or dragging anywhere on the screen to move the dot, and by flipping the \
  toggle at the bottom of the screen.
  """

struct Todo: Equatable, Identifiable {
    var description = ""
    let id: UUID
    var isComplete = false
}

enum TodoAction: Equatable {
    case checkBoxToggled
    case textFieldChanged(String)
}

struct TodoEnvironment {}

let todoReducer = Reducer<Todo, TodoAction, TodoEnvironment> { todo, action, _ in
    switch action {
    case .checkBoxToggled:
        todo.isComplete.toggle()
        return .none
        
    case let .textFieldChanged(description):
        todo.description = description
        return .none
    }
}

struct TodoView: View {
    let store: Store<Todo, TodoAction>
    
    var body: some View {
        WithViewStore(self.store) { viewStore in
            HStack {
                Button(action: { viewStore.send(.checkBoxToggled) }) {
                    Image(systemName: viewStore.isComplete ? "checkmark.square" : "square")
                }
                .buttonStyle(PlainButtonStyle())
                
                TextField(
                    "Untitled Todo",
                    text: viewStore.binding(get: \.description, send: TodoAction.textFieldChanged)
                )
            }
            .foregroundColor(viewStore.isComplete ? .gray : nil)
        }
    }
}
extension IdentifiedArray where ID == Todo.ID, Element == Todo {
    static let mock: Self = IdentifiedArray(uniqueElements: (1...10_000).map {
        Todo(description: "TODO \($0)", id: UUID(), isComplete: false)
    })
}

extension Effect where Failure == Never {
    public static func keyFrames<S>(
        values: [(output: Output, duration: S.SchedulerTimeType.Stride)],
        scheduler: S
    ) -> Effect where S: Scheduler {
        .concatenate(
            values
                .enumerated()
                .map { index, animationState in
                    index == 0
                    ? Effect(value: animationState.output)
                    : Just(animationState.output)
                        .delay(for: values[index - 1].duration, scheduler: scheduler)
                        .eraseToEffect()
                }
        )
    }
}

struct AnimationsState: Equatable {
    var alert: AlertState<AnimationsAction>? = nil
    var circleCenter = CGPoint(x: 50, y: 50)
    var circleColor = Color.white
    var isCircleScaled = false
    var todos: IdentifiedArrayOf<Todo> = .mock
}

enum AnimationsAction: Equatable {
    case circleScaleToggleChanged(Bool)
    case dismissAlert
    case rainbowButtonTapped
    case resetButtonTapped
    case resetConfirmationButtonTapped
    case setColor(Color)
    case tapped(CGPoint)
    case todo(id: Todo.ID, action: TodoAction)
}

struct AnimationsEnvironment {
    var mainQueue: AnySchedulerOf<DispatchQueue>
}


let animationsReducerCore = Reducer<AnimationsState, AnimationsAction, AnimationsEnvironment> {
    state, action, environment in
    
    switch action {
    case let .circleScaleToggleChanged(isScaled):
        state.isCircleScaled = isScaled
        return .none
        
    case .dismissAlert:
        state.alert = nil
        return .none
        
    case .rainbowButtonTapped:
        return .keyFrames(
            values: [Color.red, .blue, .green, .orange, .pink, .purple, .yellow, .white]
                .map { (output: .setColor($0), duration: 1) },
            scheduler: environment.mainQueue.animation(.linear)
        )
        
    case .resetButtonTapped:
        state.alert = .init(
            title: .init("Reset state?"),
            primaryButton: .destructive(
                .init("Reset"),
                action: .send(.resetConfirmationButtonTapped, animation: .default)
            ),
            secondaryButton: .cancel(.init("Cancel"))
        )
        return .none
        
    case .resetConfirmationButtonTapped:
        state = .init()
        return .none
        
    case let .setColor(color):
        state.circleColor = color
        return .none
        
    case let .tapped(point):
        state.circleCenter = point
        return .none
    case .todo:
        return .none
    }
}

let animationsReducer = Reducer<AnimationsState, AnimationsAction, AnimationsEnvironment>.combine(
    todoReducer.forEach(
        state: \.todos,
        action: /AnimationsAction.todo(id:action:),
        environment: { _ in TodoEnvironment() }
    ),
    animationsReducerCore
)

struct AnimationsView: View {
    @Environment(\.colorScheme) var colorScheme
    let store: Store<AnimationsState, AnimationsAction>
    
    var body: some View {
        GeometryReader { proxy in
            WithViewStore(self.store) { viewStore in
                VStack(alignment: .leading) {
                    ZStack(alignment: .center) {
                        List {
                            ForEachStore(
                                self.store.scope(state: \.todos, action: AnimationsAction.todo(id:action:)),
                                content: TodoView.init(store:)
                            )
                        }
                        Circle()
                            .fill(viewStore.circleColor)
                            .blendMode(.difference)
                            .frame(width: 50, height: 50)
                            .scaleEffect(viewStore.isCircleScaled ? 2 : 1)
                            .offset(
                                x: viewStore.circleCenter.x - proxy.size.width / 2,
                                y: viewStore.circleCenter.y - proxy.size.height / 2
                            )
                    }
                    .frame(maxWidth: .infinity, maxHeight: .infinity)
                    .background(self.colorScheme == .dark ? Color.black : .white)
                    .simultaneousGesture(
                        DragGesture(minimumDistance: 0).onChanged { gesture in
                            viewStore.send(
                                .tapped(gesture.location),
                                animation: .interactiveSpring(response: 0.25, dampingFraction: 0.1)
                            )
                        }
                    )
                    Toggle(
                        "Big mode",
                        isOn:
                            viewStore
                            .binding(get: \.isCircleScaled, send: AnimationsAction.circleScaleToggleChanged)
                            .animation(.interactiveSpring(response: 0.25, dampingFraction: 0.1))
                    )
                        .padding()
                    Button("Rainbow") { viewStore.send(.rainbowButtonTapped, animation: .linear) }
                    .padding([.leading, .trailing, .bottom])
                    Button("Reset") { viewStore.send(.resetButtonTapped) }
                    .padding([.leading, .trailing, .bottom])
                }
                .alert(self.store.scope(state: \.alert), dismiss: .dismissAlert)
            }
        }
    }
}

struct AnimationsView_Previews: PreviewProvider {
    static var previews: some View {
        Group {
            NavigationView {
                AnimationsView(
                    store: Store(
                        initialState: .init(),
                        reducer: animationsReducer,
                        environment: AnimationsEnvironment(
                            mainQueue: .main
                        )
                    )
                )
            }
            
            NavigationView {
                AnimationsView(
                    store: Store(
                        initialState: .init(),
                        reducer: animationsReducer,
                        environment: AnimationsEnvironment(
                            mainQueue: .main
                        )
                    )
                )
            }
            .environment(\.colorScheme, .dark)
        }
    }
}

Animation with ForEachStore (Modified case study)

Screen.Recording.2021-10-07.at.6.30.00.PM.mov

Animation without ForEachStore (Original case study)

Screen.Recording.2021-10-07.at.6.33.04.PM.mov

I wonder if anyone has faced similar issue or I've made any mistake in my above code, thanks!

[iampatbrown]

I'd also like to echo what @tgrapperon said regarding the simulator. I've spent ages troubleshooting animations only to find out it was working fine on a device.

[tgrapperon]

@iampatbrown I think the simulator uses macOS's SwiftUI/Core Animation implementation, which is not as efficient as on iOS (at least on Intel & Big Sur). I've also wasted a lot of time trying to improve animations on the simulator!

I've also noticed that App Store optimized builds are performing way better than debug ones. In some cases, it may be worth spending the time to test an optimized build.

[josephktcheung]

@iampatbrown Thanks for the code provided! I tried it and the performance is much better and feels like the same as the original case study in the simulator 👍 .

I think the learning here is to avoid any needless state observation in the parent which contains a ForEachStore, which would make the list child view to re-render every time the parent view state changes.

[iampatbrown]

@josephktcheung and more broadly... avoid observing state that is not used by the view. If you replaced ForEachStore(...) with ForEach(viewStore.todos) { Text("\($0.description)") } in the original example, you would still experience performance issues.

[tgrapperon]

Indeed @iampatbrown is perfectly right about avoiding observing too much state. The "code smell" here is that we're mixing the "Drag Circle" feature with the "List" feature in the same body. In a true TCA spirit, each one would live in its own domain/closed view. If some ViewStore was needed to handle some cross-domain logic, it would have to expose only these cross-domain properties. A ForEachStore (or any non-trivial component) shouldn't be able to reach any ViewStore's properties it doesn't require.

I've said some erroneous things in my previous response about the time it could take to reduce actions with forEach reducers. If the action is not from the rows' domain (like updating the drag position), it will simply be bypassed. I've updated my response accordingly.

However, there is also a lesson to learn with TextField which seems quite inefficient at receiving its binding. This is not a TCA issue though, but possibly something to keep in mind when working with many TextFields.