Skip to content

Commit ce356b7

Browse files
committed
[perf] improve ComposeNodeId speed
1 parent 435f66c commit ce356b7

11 files changed

Lines changed: 254 additions & 29 deletions

ComposeUI/Sources/ComposeUI/ComposeNode/ComposeNode.swift

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,7 @@ public protocol ComposeNode: ComposeContent {
4343
///
4444
/// The id must be unique for a node type.
4545
///
46-
/// For your own node type, don't use `.standard` id. Instead, use `.custom`
47-
/// with a prefix to avoid conflicts.
46+
/// For your own node type, use `.custom` with a prefix to avoid conflicts.
4847
///
4948
/// For example, if you have a `MyCustomNode`, you can use id like
5049
/// `.custom("com.myapp.mycustomnode", isFixed: false)`.
@@ -122,7 +121,7 @@ public extension ComposeNode {
122121
}
123122

124123
private func id(_ id: ComposeNodeId) -> Self {
125-
guard self.id != id else {
124+
guard !self.id.isSameConfiguration(as: id) else {
126125
return self
127126
}
128127

ComposeUI/Sources/ComposeUI/ComposeNode/ComposeNodeId.swift

Lines changed: 81 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ enum StandardComposeNodeId: String {
5959
case composeView = "CV"
6060
}
6161

62-
public struct ComposeNodeId: Equatable {
62+
public struct ComposeNodeId: Hashable {
6363

6464
/// Create a custom id for a node.
6565
///
@@ -81,13 +81,75 @@ public struct ComposeNodeId: Equatable {
8181
}
8282

8383
/// The id of the node.
84-
let id: String
84+
public let id: String
8585

8686
/// If the id is fixed.
8787
///
8888
/// If the id is fixed, `join` will not add the parent node's id to the child node's id.
8989
private let isFixed: Bool
9090

91+
/// A precomputed 64-bit hash of `id`.
92+
///
93+
/// `ComposeView` uses node ids as render-pass dictionary/set keys. Caching this value keeps dictionary probes from
94+
/// repeatedly hashing long composed id strings, while equality still checks the exact string so collisions remain safe.
95+
private let cachedHash: UInt64
96+
97+
private init(id: String, isFixed: Bool) {
98+
self.id = id
99+
self.isFixed = isFixed
100+
self.cachedHash = Self.fnv1a(id)
101+
}
102+
103+
private init(id: String, isFixed: Bool, cachedHash: UInt64) {
104+
self.id = id
105+
self.isFixed = isFixed
106+
self.cachedHash = cachedHash
107+
}
108+
109+
// MARK: - Hashable
110+
111+
public func hash(into hasher: inout Hasher) {
112+
hasher.combine(cachedHash)
113+
}
114+
115+
public static func == (lhs: ComposeNodeId, rhs: ComposeNodeId) -> Bool {
116+
// A node id's identity is its composed `id` string. `isFixed` only controls how the id is composed (whether a parent
117+
// prefix is added in `join`), not which item the id denotes, so it is intentionally excluded from equality. This
118+
// matches the render diff's long-standing behavior of keying its maps by the id string only: a `fixedId("x")` item
119+
// and a composed item that resolves to "x" are the same (conflicting) render item, not two distinct ones.
120+
//
121+
// The cheap precomputed hash is compared first to reject mismatches without touching the strings; the exact `id`
122+
// compare then makes equality collision-free (two distinct ids that happen to share a hash are never treated equal).
123+
lhs.cachedHash == rhs.cachedHash && lhs.id == rhs.id
124+
}
125+
126+
func isSameConfiguration(as other: ComposeNodeId) -> Bool {
127+
id == other.id && isFixed == other.isFixed
128+
}
129+
130+
/// FNV-1a hash of the string's UTF-8 bytes.
131+
///
132+
/// Used to derive `cachedHash` from `id`, so the same `id` always yields the same hash regardless of how it was
133+
/// composed (keeping `Hashable` consistent with the exact `==`).
134+
@inline(__always)
135+
private static func fnv1a(_ string: String) -> UInt64 {
136+
fnv1a(string, hash: HashConstants.fnvOffsetBasis)
137+
}
138+
139+
@inline(__always)
140+
private static func fnv1a(_ string: String, hash initialHash: UInt64) -> UInt64 {
141+
var hash = initialHash
142+
for byte in string.utf8 {
143+
hash = fnv1a(byte, hash: hash)
144+
}
145+
return hash
146+
}
147+
148+
@inline(__always)
149+
private static func fnv1a(_ byte: UInt8, hash: UInt64) -> UInt64 {
150+
(hash ^ UInt64(byte)) &* HashConstants.fnvPrime
151+
}
152+
91153
/// Make a `ComposeNodeId` by joining the current node's id with a child node's id.
92154
///
93155
/// If the `childNodeId` is fixed, it will return the `childNodeId` directly.
@@ -101,10 +163,25 @@ public struct ComposeNodeId: Equatable {
101163
return childNodeId
102164
} else {
103165
if let suffix {
104-
return ComposeNodeId(id: "\(id)|\(suffix)|\(childNodeId.id)", isFixed: isFixed)
166+
var hash = Self.fnv1a(HashConstants.separator, hash: cachedHash)
167+
hash = Self.fnv1a(suffix, hash: hash)
168+
hash = Self.fnv1a(HashConstants.separator, hash: hash)
169+
hash = Self.fnv1a(childNodeId.id, hash: hash)
170+
return ComposeNodeId(id: "\(id)|\(suffix)|\(childNodeId.id)", isFixed: isFixed, cachedHash: hash)
105171
} else {
106-
return ComposeNodeId(id: "\(id)|\(childNodeId.id)", isFixed: isFixed)
172+
var hash = Self.fnv1a(HashConstants.separator, hash: cachedHash)
173+
hash = Self.fnv1a(childNodeId.id, hash: hash)
174+
return ComposeNodeId(id: "\(id)|\(childNodeId.id)", isFixed: isFixed, cachedHash: hash)
107175
}
108176
}
109177
}
110178
}
179+
180+
// MARK: - Constants
181+
182+
private enum HashConstants {
183+
184+
static let separator: UInt8 = 0x7C // "|"
185+
static let fnvOffsetBasis: UInt64 = 0xCBF29CE484222325
186+
static let fnvPrime: UInt64 = 0x00000100000001B3
187+
}

ComposeUI/Sources/ComposeUI/ComposeView/ComposeView+Debug.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ public extension ComposeView {
8585
case renderDidInsertRenderable(item: RenderableItem, renderable: Renderable)
8686

8787
/// The render pass is finished, all the renderable items are rendered, transition animations or animations may still be running.
88-
case renderDidFinish(renderableItemIds: [String], renderableItemMap: [String: RenderableItem], renderableMap: [String: Renderable])
88+
case renderDidFinish(renderableItemIds: [ComposeNodeId], renderableItemMap: [ComposeNodeId: RenderableItem], renderableMap: [ComposeNodeId: Renderable])
8989
}
9090

9191
private weak var composeView: ComposeView?

ComposeUI/Sources/ComposeUI/ComposeView/ComposeView+ZOrder.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ extension ComposeView {
6969
/// - newIds: The renderable item ids of the current render pass, in z-order (back to front).
7070
/// - reusingIds: The ids that are in both `oldIds` and `newIds`.
7171
/// - Returns: The z-order plan.
72-
static func makeZOrderPlan(oldIds: [String], newIds: [String], reusingIds: Set<String>) -> ZOrderPlan {
72+
static func makeZOrderPlan(oldIds: [ComposeNodeId], newIds: [ComposeNodeId], reusingIds: Set<ComposeNodeId>) -> ZOrderPlan {
7373
// when the content is unchanged, no z-order maintenance is needed.
7474
if newIds == oldIds {
7575
return .minimal(needsNewItemPlacement: false)
@@ -139,7 +139,7 @@ extension ComposeView {
139139
/// - reusingIds: The ids of the retained renderables.
140140
/// - renderableItemIds: The ids of the renderables being rendered, in z-order (back to front).
141141
/// - renderableMap: The map of the renderables being rendered, keyed by id.
142-
func placeNewRenderables(reusingIds: Set<String>, renderableItemIds: [String], renderableMap: [String: Renderable]) {
142+
func placeNewRenderables(reusingIds: Set<ComposeNodeId>, renderableItemIds: [ComposeNodeId], renderableMap: [ComposeNodeId: Renderable]) {
143143
let parent: View = contentView()
144144

145145
// the next sibling of the same kind (already placed correctly), while walking from front to back

ComposeUI/Sources/ComposeUI/ComposeView/ComposeView.swift

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -121,21 +121,21 @@ open class ComposeView: BaseScrollView {
121121
private var lastRenderBounds: CGRect = .zero
122122

123123
/// The ids of the renderable items that are being rendered.
124-
private var renderableItemIds: [String] = []
124+
private var renderableItemIds: [ComposeNodeId] = []
125125

126126
/// The map of the renderable items that are being rendered.
127-
private var renderableItemMap: [String: RenderableItem] = [:]
127+
private var renderableItemMap: [ComposeNodeId: RenderableItem] = [:]
128128

129129
/// The map of the renderables that are being rendered.
130-
private var renderableMap: [String: Renderable] = [:]
130+
private var renderableMap: [ComposeNodeId: Renderable] = [:]
131131

132132
/// The map of the renderables that are being removed.
133133
///
134134
/// The removing renderables are the ones that are not in the renderable hierarchy but still rendered due to the transition.
135-
private var removingRenderableMap: [String: Renderable] = [:]
135+
private var removingRenderableMap: [ComposeNodeId: Renderable] = [:]
136136

137137
/// The map of the removing renderable transition completion blocks.
138-
private var removingRenderableTransitionCompletionMap: [String: CancellableBlock] = [:]
138+
private var removingRenderableTransitionCompletionMap: [ComposeNodeId: CancellableBlock] = [:]
139139

140140
/// The pool that recycles renderables across render passes.
141141
///
@@ -903,8 +903,8 @@ open class ComposeView: BaseScrollView {
903903
ComposeUI.assert(oldRenderableItemIds.count == oldRenderableItemMap.count, "mismatched old renderable item count")
904904
ComposeUI.assert(oldRenderableItemIds.count == oldRenderableMap.count, "mismatched old renderable count")
905905
for id in oldRenderableItemIds {
906-
ComposeUI.assert(oldRenderableItemMap[id] != nil, "missing old renderable item: \(id)")
907-
ComposeUI.assert(oldRenderableMap[id] != nil, "missing old renderable: \(id)")
906+
ComposeUI.assert(oldRenderableItemMap[id] != nil, "missing old renderable item: \(id.id)")
907+
ComposeUI.assert(oldRenderableMap[id] != nil, "missing old renderable: \(id.id)")
908908
}
909909
#endif
910910

@@ -917,14 +917,14 @@ open class ComposeView: BaseScrollView {
917917
renderableMap.reserveCapacity(renderableItemsCount)
918918

919919
for item in renderableItems {
920-
let id = item.id.id
921-
ComposeUI.assert(renderableItemMap[id] == nil, "conflicting renderable item id: \(id)")
920+
let id = item.id
921+
ComposeUI.assert(renderableItemMap[id] == nil, "conflicting renderable item id: \(id.id)")
922922
renderableItemIds.append(id)
923923
renderableItemMap[id] = item
924924
}
925925

926926
// update the renderables
927-
var reusingIds: Set<String> = []
927+
var reusingIds: Set<ComposeNodeId> = []
928928

929929
for oldId in oldRenderableItemIds {
930930
if renderableItemMap[oldId] == nil {
@@ -1012,7 +1012,7 @@ open class ComposeView: BaseScrollView {
10121012
#endif
10131013
}
10141014
} else {
1015-
ComposeUI.assertFailure("old renderable item or old renderable not found: \(oldId)")
1015+
ComposeUI.assertFailure("old renderable item or old renderable not found: \(oldId.id)")
10161016
}
10171017
} else {
10181018
// this renderable item is still in the content, plan to reuse it
@@ -1191,8 +1191,8 @@ open class ComposeView: BaseScrollView {
11911191
ComposeUI.assert(renderableItemIds.count == renderableItemMap.count, "mismatched renderable item count")
11921192
ComposeUI.assert(renderableItemIds.count == renderableMap.count, "mismatched renderable count")
11931193
for id in renderableItemIds {
1194-
ComposeUI.assert(renderableItemMap[id] != nil, "missing renderable item: \(id)")
1195-
ComposeUI.assert(renderableMap[id] != nil, "missing renderable: \(id)")
1194+
ComposeUI.assert(renderableItemMap[id] != nil, "missing renderable item: \(id.id)")
1195+
ComposeUI.assert(renderableMap[id] != nil, "missing renderable: \(id.id)")
11961196
}
11971197
#endif
11981198

@@ -1255,11 +1255,11 @@ open class ComposeView: BaseScrollView {
12551255
host.contentUpdateContext
12561256
}
12571257

1258-
var removingRenderableMap: [String: Renderable] {
1258+
var removingRenderableMap: [ComposeNodeId: Renderable] {
12591259
host.removingRenderableMap
12601260
}
12611261

1262-
var removingRenderableTransitionCompletionMap: [String: CancellableBlock] {
1262+
var removingRenderableTransitionCompletionMap: [ComposeNodeId: CancellableBlock] {
12631263
host.removingRenderableTransitionCompletionMap
12641264
}
12651265
}

ComposeUI/Tests/ComposeUITests/ComposeNode/ComposeNodeIdTests.swift

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,23 @@ class ComposeNodeIdTests: XCTestCase {
3939
expect(id.id) == "C"
4040
}
4141

42+
func test_identity_ignoresIsFixed() {
43+
let nonFixed = ComposeNodeId.custom("x", isFixed: false)
44+
let fixed = ComposeNodeId.custom("x", isFixed: true)
45+
46+
expect(nonFixed) == fixed
47+
expect(nonFixed.hashValue) == fixed.hashValue
48+
49+
var map: [ComposeNodeId: Int] = [:]
50+
map[nonFixed] = 1
51+
map[fixed] = 2
52+
expect(map.count) == 1
53+
expect(map[nonFixed]) == 2
54+
55+
// different id strings remain distinct.
56+
expect(ComposeNodeId.custom("x")) != ComposeNodeId.custom("y")
57+
}
58+
4259
func test_custom() {
4360
let id = ComposeNodeId.custom("test")
4461
expect(id.id) == "test"
@@ -54,6 +71,46 @@ class ComposeNodeIdTests: XCTestCase {
5471
}
5572
}
5673

74+
func test_join_hashMatchesDirectId() {
75+
let parentId = ComposeNodeId.custom("parent")
76+
let childId = ComposeNodeId.custom("test")
77+
78+
do {
79+
let joinedId = parentId.join(with: childId)
80+
let directId = ComposeNodeId.custom("parent|test")
81+
expect(joinedId) == directId
82+
expect(joinedId.hashValue) == directId.hashValue
83+
84+
var map: [ComposeNodeId: Int] = [:]
85+
map[joinedId] = 1
86+
map[directId] = 2
87+
expect(map.count) == 1
88+
expect(map[joinedId]) == 2
89+
}
90+
91+
do {
92+
let joinedId = parentId.join(with: childId, suffix: "suffix")
93+
let directId = ComposeNodeId.custom("parent|suffix|test")
94+
expect(joinedId) == directId
95+
expect(joinedId.hashValue) == directId.hashValue
96+
}
97+
}
98+
99+
func test_join_deepHashMatchesDirectId() {
100+
var id = ComposeNodeId.custom("root")
101+
var expectedId = "root"
102+
103+
for index in 0 ..< 20 {
104+
let childId = ComposeNodeId.custom("child-\(index)")
105+
id = id.join(with: childId, suffix: "suffix-\(index)")
106+
expectedId += "|suffix-\(index)|child-\(index)"
107+
}
108+
109+
let directId = ComposeNodeId.custom(expectedId)
110+
expect(id) == directId
111+
expect(id.hashValue) == directId.hashValue
112+
}
113+
57114
func test_custom_fixed() {
58115
let id = ComposeNodeId.custom("test", isFixed: true)
59116
expect(id.id) == "test"

ComposeUI/Tests/ComposeUITests/ComposeNode/ComposeNodeTests.swift

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,13 +45,20 @@ class ComposeNodeTests: XCTestCase {
4545
do {
4646
let node = MockComposeNode().id("test")
4747
expect(node.id) == .custom("test", isFixed: false)
48+
expect(node.id.isSameConfiguration(as: .custom("test", isFixed: false))) == true
4849
}
4950

5051
do {
5152
let node = MockComposeNode().fixedId("test")
5253
expect(node.id) == .custom("test", isFixed: true)
54+
expect(node.id.isSameConfiguration(as: .custom("test", isFixed: true))) == true
5355
}
5456
}
57+
58+
func test_fixedId_appliesWhenIdStringUnchanged() {
59+
let node = MockComposeNode().id("test").fixedId("test")
60+
expect(ComposeNodeId.custom("parent").join(with: node.id).id) == "test"
61+
}
5562
}
5663

5764
private class MockComposeNode: ComposeNode {

ComposeUI/Tests/ComposeUITests/ComposeView/ComposeView+RenderReuseTests.swift

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,41 @@ class ComposeView_RenderReuseTests: XCTestCase {
8888
expect(counter.madeCount) > initialMakeCount
8989
}
9090

91+
// MARK: - Render identity
92+
93+
func test_renderIdentity_ignoresIsFixed_reusesAcrossIsFixedChange() {
94+
// A node id'd "x" (non-fixed) and the same node `fixedId`'d "x" are the SAME render item, because render identity is
95+
// the id string only. Changing only the "isFixed" must therefore reuse the existing renderable in place rather than
96+
// remove the old one and create a new one (which is what would happen if `isFixed` were part of the render map keys).
97+
var madeCount = 0
98+
let view = ComposeView(frame: CGRect(origin: .zero, size: Constants.viewSize))
99+
view.renderablePool = nil // isolate map-based reuse from the recycle pool
100+
101+
view.setContent {
102+
LayerNode(make: { _ in
103+
madeCount += 1
104+
return CALayer()
105+
})
106+
.id("x")
107+
}
108+
view.refresh(animated: false)
109+
expect(madeCount) == 1
110+
111+
// same string id, now fixed: the render maps (renderableItemMap/renderableMap) must treat it as the same key.
112+
view.setContent {
113+
LayerNode(make: { _ in
114+
madeCount += 1
115+
return CALayer()
116+
})
117+
.fixedId("x")
118+
}
119+
view.refresh(animated: false)
120+
121+
// reused in place: no new renderable was created, and nothing was stranded in the removing map.
122+
expect(madeCount) == 1
123+
expect(view.test.removingRenderableMap.count) == 0
124+
}
125+
91126
// MARK: - Default pooling (common nodes)
92127

93128
func test_colorNode_poolsByDefault_acrossScroll() {

0 commit comments

Comments
 (0)