Change StoreProtocol to implement transaction

[gordonbrander]

...rather than send.

• transact(:) is a non-isolated method that performs a synchronous state transaction in response to an action.
• send(:) becomes an asynchronous main actor isolated protocol extension that guarantees your changes will always happen on the main thread, but offers only a fire-and-forget interface.

[gordonbrander]

@bfollington here's my take on #46.

Not sure if the method rename will cause too much carnage in Subconscious. If so, we could instead do:

• send(:) synchronous but not isolated to main thread
• sendAsync(:)

The tradeoff would be that we have to manually go change .send() to .sendAsync() in these cases.

The constraints we're working with:

• SwiftUI requires state to change on main thread
• However none of the SwiftUI APIs are main actor isolated, (Binding, etc)
• Binding expects a synchronous mutation (pretty sure about this)

As a result:

• Need a send-like method, transact(:), that is not isolated, but expected to be called only on main thread. We use this to implement the bridge to Binding.
• Need a send-like method, send(:) that is fire-and-forget, and calls transact(:) within a MainActor isolated task.

[bfollington]

Nice! This is very similar to (one of) the abstractions I tried, it looks good on paper:

1. correct thread assignment
2. tests pass
3. actions and effects occur as expected

BUT it seems the major performance penalty comes from the synchronous mutation of a binding. Comparing the attached code snippets, the first one has a noticeable hang (visible in profiling tools) on each tab transition and the second does not.

A - has hangs

TabView(
    selection: Binding(
        get: { store.state.selectedAppTab },
        transact: store.transact,
        tag: AppAction.setSelectedAppTab
    )
)

B - no hangs

TabView(
    selection: Binding(
        get: { store.state.selectedAppTab },
        transact: { action in Task { store.transact(action) } },
        tag: AppAction.setSelectedAppTab
    )
)

Now, this violates a bunch of runtime guarantees so it's not acceptable. I think, if we want safe bindings + good performance, we might have to pull a similar trick to ValidatedTextField where we have a local @State variable powering the binding and replay the changes to the store in the background:

C - proxy binding

@State var selectedTab: AppTab = .notebook

var body: some View {
    TabView(
        selection: $selectedTab
    ) { ... }
    .onAppear {
        selectedTab = store.state.selectedAppTab
    }
    .onChange(of: selectedTab) { v in
        store.send(.setSelectedAppTab(selectedTab))
    }
}

This has the best performance I've seen yet in the profiler and suggests we should avoid ever directly calling store.transact in a Binding.

I agree with your assessment that bindings seem to expect synchronous updates but we seem to be doing WAY too much work on each update to support that (because we touch so much of the tree on every change to every store).

[bfollington]

You can somewhat bundle this into a ViewModifier:

@State var selectedTab: AppTab = .notebook

var body: some View {
    TabView(selection: $selectedTab) { ... }
    .modifier(
        StoreProxyViewModifier(
            state: $selectedTab,
            get: { store.state.selectedAppTab },
            send: store.send,
            tag: AppAction.setSelectedAppTab
        )
    )
}

I also had a go at writing a custom @propertyWrapper that could encapsulate this but I don't think I get the API I want.

[bfollington]

I think we should go ahead and land this, along with subconsciousnetwork/subconscious#1124. They make a considerable improvement to performance, even if we know we need to go deeper on this problem in the long run.