Utilize cache infra #587
Utilize cache infra #587
Conversation
* maxg/cache_1_equiv: Fix testkey
* maxg/cache_2_envcache: Fix testkey
* maxg/cache_2_envcache: Fix testkey
* maxg/cache_2_envcache: Fix testkey
* maxg/cache_1_equivalency: include other TestKeys Fix testkey
* maxg/cache_2_envcache: include other TestKeys Fix testkey
* maxg/cache_2_envcache: include other TestKeys Fix testkey
* maxg/cache_2_envcache: include other TestKeys Fix testkey
* maxg/cache_1_equivalency: Cleanup
* maxg/cache_2_envcache: Merge. Cleanup
* maxg/cache_2_envcache: More tests Tests and fixes
* maxg/cache_2_envcache: Tweak env tests More tests
* maxg/cache_2_envcache: Existentials for Xcode 15
* maxg/cache_2_envcache: Enable logging for test. Tweak internal env api. Log guard
* maxg/cache_2_envcache: Release 6.3.0 Assert in debug mode when large content viewer is not placed inside an interaction container Expose largeContentViewerInteraction on LargeContentViewer backing view via protocol Stopped installing xcodes in GitHub Actions file chore: Release 6.2.0 Support accessibility large content viewer Squash
There was a problem hiding this comment.
Can you rebase this on the maxg/cache_2_envcache branch? Looks like it's got some changes from the other 2 stacked PRs mixed in.
| // Shortcut: If both environments were empty, nothing changed. | ||
| if oldValue.isEmpty && environment.isEmpty { return } | ||
| // Shortcut: If there are no changes to the environment, then, well, nothing changed. | ||
| if let layoutMode, layoutMode.options.skipUnneededSetNeedsViewHierarchyUpdates && oldValue.isEquivalent( |
There was a problem hiding this comment.
self.layoutMode is just one customization point — you should probably use something like:
self.layoutMode ?? RenderContext.current?.layoutMode ?? environment.layoutMode
which I think will match the actual mode used during layout.
| return measurer(proposal, environment) | ||
| } | ||
|
|
||
| let key = MeasurableSizeKey(path: node.path, max: proposal.maximum) |
There was a problem hiding this comment.
better to use proposal directly than maximum I think — the latter is a headache to deal with and something I'd like to deprecate
| /// - parameter validationKey: If present, measureFunction will attempt to cache sizing based on the path of the node. validationKey will be evaluated to ensure that the result is valid. | ||
| /// - parameter measureFunction: How to measure the `ElementContent` in the given `SizeConstraint` and `Environment`. | ||
| public init( | ||
| validationKey: some ContextuallyEquivalent, |
There was a problem hiding this comment.
pitching name measureCachingKey over validationKey — as a consumer there's no "validation" aspect to it, it's used to cache measurements
| // | ||
| // Created by Max Goedjen on 7/23/25. | ||
| // | ||
|
|
| // Store bounding shapes in this cache to avoid costly recalculations | ||
| private var boundingShapeCache: [Link: Link.BoundingShape] = [:] | ||
|
|
There was a problem hiding this comment.
what happened to the deleted stuff in this file? intentional change, or just a bad merge?
|
|
||
| self.element = element | ||
| self.environment = environment | ||
| self.environment.cacheStorage = cacheStorage |
There was a problem hiding this comment.
shouldn't need to set this here, it'll be applied during makeEnvironment. self.environment is the "base"
3/3 for caching changes.
Design doc: https://docs.google.com/document/d/1CLhFbzZGbEKgvZTUwLuGbFY-rfZNJVTi6f3PkoZT9qI/edit?usp=sharing
Adds support for caching AttributedLabel string normalization results.
Adds support for skipping calls to setNeedsViewHierarchyUpdate when Environment and Element value changes are equivalent.
Adds support for caching MeasureableStorage measurements.