Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Overhaul store scope communication #2664

Merged
merged 57 commits into from
Dec 20, 2023
Merged

Overhaul store scope communication #2664

merged 57 commits into from
Dec 20, 2023

Conversation

stephencelis
Copy link
Member

With the recent introduction of child store caching (#2527) the performance of re-scoping stores dramatically improved, but the lifetime of scoped stores has also been extended (sometimes to the lifetime of the application), which can lead to performance issues of its own. In particular, the current model of scoping spins off a child store that maintains its own state and reducer, subscribes to its parent for state updates, and communicates actions back up to its parent when its reducer receives one. Because child stores are no longer created and destroyed regularly alongside views as they appear and disappear, more and more active subscriptions to parent stores can begin to bog down an application.

This PR attempts to alleviate this issue by rethinking the parent/child relationship of scoped stores. Rather than thinking of scoping as an operation that synchronizes child with parent, we can think of all scoped stores as simple views on a root store, where accessing state is a matter of key-pathing from some root state to some child state, and sending actions is a matter of embedding some child action in a root before sending it along. This means we can eliminate all child store subscriptions: only view stores need to subscribe to updates in order to ping their objectWillChange publisher.

Note that store scoping was previously a flat stack operation due to the fact that all child stores maintained their own local copy of state. This is no longer the case, but store scoping can remain flat via key path-scoping, which is stackless: we can append key paths at each layer of scoping rather than accumulate another stack frame in a nested closure. This does mean that closure-based scopes will incur additional stack frames when adopting this code, so applications with large root states should update to use key path-based scoping wherever possible before adopting these changes.

@acosmicflamingo
Copy link
Contributor

image

With the changes in this PR, the issue in https://github.com/acosmicflamingo/StoreScopingCacheReproducer disappears.

[StoreScopingCacheReproducerUITests.StoreScopingCacheReproducerUITests testSectionHeaderText]' passed (5.405 seconds).
Test Suite 'StoreScopingCacheReproducerUITests' passed at 2023-12-17 18:30:04.010.
	 Executed 100 tests, with 0 failures (0 unexpected) in 532.213 (532.288) seconds

Additionally, the section header issue that was going on in my own music app also goes away with this change (and I really tried my best to break it). Now this is a real Christmas miracle!

Comment on lines +630 to +632
The library provides a tool to perform these steps in a single step. It's the
``PresentationState/subscript(case:)-7uqte`` defined on ``PresentationState`` which allows you to
modify the data inside a case of the destination enum:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was unrelated to this PR, but I just realized we should probably be pushing people towards the [case:] modify subscript instead of XCTModify by default. XCTModify is really only handy if you want to perform a bunch of mutations on the same case.

@stephencelis stephencelis marked this pull request as ready for review December 18, 2023 23:02
@stephencelis stephencelis merged commit 087b6b5 into main Dec 20, 2023
5 checks passed
@stephencelis stephencelis deleted the store2 branch December 20, 2023 02:35
@stephencelis stephencelis restored the store2 branch December 20, 2023 02:36
cgrindel-self-hosted-renovate bot referenced this pull request in cgrindel/rules_swift_package_manager Dec 21, 2023
…ure to from: "1.5.6" (#821)

This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
|
[pointfreeco/swift-composable-architecture](https://togithub.com/pointfreeco/swift-composable-architecture)
| patch | `from: "1.5.5"` -> `from: "1.5.6"` |

---

### Release Notes

<details>
<summary>pointfreeco/swift-composable-architecture
(pointfreeco/swift-composable-architecture)</summary>

###
[`v1.5.6`](https://togithub.com/pointfreeco/swift-composable-architecture/releases/tag/1.5.6)

[Compare
Source](https://togithub.com/pointfreeco/swift-composable-architecture/compare/1.5.5...1.5.6)

#### What's Changed

> \[!IMPORTANT]
> While this release contains no additions or breaking changes to the
Composable Architecture's APIs, the `Store.scope` operation has been
significantly refactored for performance. While we have vetted these
changes in our own test suites and applications, and have worked with
members of the community to test these changes before this release,
please thoroughly test the view layer of your applications after
upgrading to this version, and before releasing your application to
production.
>
> See
[#&#8203;2664](https://togithub.com/pointfreeco/swift-composable-architecture/issues/2664)
for more details on the change.

- Performance: Overhaul store scope communication
([https://github.com/pointfreeco/swift-composable-architecture/pull/2664](https://togithub.com/pointfreeco/swift-composable-architecture/pull/2664)).
- Infrastructure: Update sample code to use the `@DependencyClient`
macro
([https://github.com/pointfreeco/swift-composable-architecture/pull/2653](https://togithub.com/pointfreeco/swift-composable-architecture/pull/2653)).
- Infrastructure: Update migration guide with more information about
applying `@CasePathable` to enums
([https://github.com/pointfreeco/swift-composable-architecture/pull/2672](https://togithub.com/pointfreeco/swift-composable-architecture/pull/2672)).
- Infrastructure: Add basics of composition to the "essentials" tutorial
([https://github.com/pointfreeco/swift-composable-architecture/pull/2659](https://togithub.com/pointfreeco/swift-composable-architecture/pull/2659)).

**Full Changelog**:
pointfreeco/swift-composable-architecture@1.5.5...1.5.6

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the
rebase/retry checkbox.

👻 **Immortal**: This PR will be recreated if closed unmerged. Get
[config help](https://togithub.com/renovatebot/renovate/discussions) if
that's undesired.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Renovate
Bot](https://togithub.com/renovatebot/renovate).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi4xMDAuMCIsInVwZGF0ZWRJblZlciI6IjM2LjEwMC4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->

Co-authored-by: Self-hosted Renovate Bot <361546+cgrindel-self-hosted-renovate[bot]@users.noreply.github.enterprise.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants