Skip to content

Commit eab21cb

Browse files
stackotterstackotter
committed
Greatly simplify state update merging implementation with a semaphore, a serial queue, and no unsafe shared state
This implementation is much nicer, hopefully it makes a difference on Rocky Linux 8 too. The main reason that I'm hoping that it can make a difference is that it avoids blocking operations entirely.
1 parent 55ff38a commit eab21cb

File tree

1 file changed

+17
-85
lines changed

1 file changed

+17
-85
lines changed

‎Sources/SwiftCrossUI/State/Publisher.swift‎

Lines changed: 17 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -73,97 +73,29 @@ public class Publisher {
7373
backend: Backend,
7474
action closure: @escaping () -> Void
7575
) -> Cancellable {
76-
// All of the concurrency related code is there to detect when updates can be merged
77-
// together (a.k.a. when one of the updates is unnecessary).
78-
let protectingQueue = DispatchQueue(label: "state update merging")
79-
let concurrentUpdateHandlingQueue = DispatchQueue(
80-
label: "concurrent update handling queue",
81-
attributes: .concurrent
76+
let serialUpdateHandlingQueue = DispatchQueue(
77+
label: "serial update handling"
8278
)
83-
let synchronizationSemaphore = DispatchSemaphore(value: 1)
84-
85-
// State shared betwen all calls to the closure defined below.
86-
var updateIsQueued = false
87-
var updateIsRunning = false
88-
var aCurrentJobDidntHaveToWait = false
89-
79+
let semaphore = DispatchSemaphore(value: 1)
9080
return observe {
91-
// I'm sorry if you have to make sense of this... Take my comments as a peace offering.
92-
93-
// Hop to a dispatch queue to avoid blocking any threads in the Swift Structured
94-
// Concurrency thread pool in the case that the state update originated from a task.
95-
concurrentUpdateHandlingQueue.async {
96-
// If no one is running, then we run without waiting, and if someone's running
97-
// but no one's waiting, then we wait and prevent anyone else from waiting.
98-
// This ensures that at least one update will always happen after every update
99-
// received so far, without letting unnecessary updates queue up. The reason
100-
// that we can merge updates like this is that all state updates are built equal;
101-
// they don't carry any information other than that they happened.
102-
var shouldWait = false
103-
protectingQueue.sync {
104-
if !updateIsQueued {
105-
shouldWait = true
106-
}
107-
108-
if updateIsRunning {
109-
updateIsQueued = true
110-
} else {
111-
updateIsRunning = true
112-
aCurrentJobDidntHaveToWait = true
113-
}
114-
}
115-
116-
guard shouldWait else {
117-
return
118-
}
81+
// Only allow one update to wait at a time.
82+
guard semaphore.wait(timeout: .now()) == .success else {
83+
return
84+
}
11985

120-
// Waiting just involves attempting to jump to the main thread.
86+
// Add update to queue. We use our own serial update handling queue since some
87+
// backends don't have the concept of a main thread, leading to the possibility
88+
// that two updates can run at once which would be inefficient and lead to
89+
// incorrect results anyway.
90+
serialUpdateHandlingQueue.async {
12191
backend.runInMainThread {
122-
// This semaphore is used because some backends don't put us on the main
123-
// thread since they don't have the concept of a single UI thread like
124-
// macOS does.
125-
//
126-
// If `backend.runInMainThread` is truly putting us on the main thread,
127-
// then this never have to block significantly, otherwise we're just
128-
// blocking some random thread, so either way we're fine since we've
129-
// explicitly hopped to a dispatch queue to escape any cooperative
130-
// Swift Structured Concurrency thread pool the state update may have
131-
// originated from.
132-
synchronizationSemaphore.wait()
133-
134-
protectingQueue.sync {
135-
// If a current job didn't have to wait, then that's us. Due to
136-
// concurrency that doesn't mean we were the first update triggered.
137-
// That is, we could've been the job that set `updateIsQueued` to
138-
// true while still being the job that reached this line first (before
139-
// the one that set `updateIsRunning` to true). And that's why I've
140-
// implemented the check in this way with a protected 'global' and not
141-
// a local variable (being first isn't a property we can know ahead
142-
// of time). I use 'global' in the sense of shared between all calls
143-
// to the state update handling closure for a given ViewGraphNode.
144-
//
145-
// The reason that `aCurrentJobDidntHaveToWait` is needed at all is
146-
// so that we can know whether `updateIsQueued`'s value is due to us
147-
// or someone else/no one.
148-
if aCurrentJobDidntHaveToWait {
149-
aCurrentJobDidntHaveToWait = false
150-
} else {
151-
updateIsQueued = false
152-
}
153-
}
92+
// Now that we're about to start, let another update queue up. If we
93+
// instead waited until we're finished the update, we'd introduce the
94+
// possibility of dropping updates that would've affected views that
95+
// we've already processed, leading to stale view contents.
96+
semaphore.signal()
15497

15598
closure()
156-
157-
// If someone is waiting then we leave `updateIsRunning` equal to true
158-
// because they'll immediately begin running as soon as we exit and we
159-
// don't want an extra person queueing until they've actually started.
160-
protectingQueue.sync {
161-
if !updateIsQueued {
162-
updateIsRunning = false
163-
}
164-
}
165-
166-
synchronizationSemaphore.signal()
16799
}
168100
}
169101
}

0 commit comments

Comments
 (0)