Skip to content
Merged
6 changes: 3 additions & 3 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,14 @@ jobs:
- uses: actions/cache@v4
with:
path: Tools/bin
key: spm-${{ runner.os }}-${{env.DEVELOPER_DIR}}-${{ hashFiles('Package.swift') }}
key: spm-${{ runner.os }}-${{env.DEVELOPER_DIR}}-${{ hashFiles('Tools/Package.swift') }}
- name: Validate lint
run: make lint
- name: Validate format
run: |
make format
if [ -n "$(git status --porcelain)" ]; then git diff --name-only && echo "Make sure that the code is formated by 'make format'."; exit 1; fi
if [ -n "$(git status --porcelain)" ]; then git diff && echo "Make sure that the code is formated by 'make format'."; exit 1; fi
- name: Validate example project
run: |
make proj
if [ -n "$(git status --porcelain)" ]; then git diff --name-only && echo "Make sure that 'Examples/App.xcodeproj' is formated by 'make proj'."; exit 1; fi
if [ -n "$(git status --porcelain)" ]; then git diff && echo "Make sure that 'Examples/App.xcodeproj' is formated by 'make proj'."; exit 1; fi
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ SWIFT_FILE_PATHS = Package.swift Tools/Package.swift Sources Tests Examples

.PHONY: proj
proj:
SWIFT_PACKAGE_RESOURCES=.build/checkouts/XcodeGen/SettingPresets $(TOOL) xcodegen -s Examples/project.yml
SWIFT_PACKAGE_RESOURCES=Tools/.build/checkouts/XcodeGen/SettingPresets $(TOOL) xcodegen -s Examples/project.yml

.PHONY: format
format:
Expand Down
10 changes: 5 additions & 5 deletions Sources/Atoms/Core/Loader/AtomLoader.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,18 @@ public protocol AtomLoader {

/// Returns a boolean value indicating whether it should notify updates downstream
/// by checking the equivalence of the given old value and new value.
func shouldUpdate(newValue: Value, oldValue: Value) -> Bool
func shouldUpdateTransitively(newValue: Value, oldValue: Value) -> Bool

/// Performs atom update.
func performUpdate(_ body: () -> Void)
/// Performs transitive update for dependent atoms.
func performTransitiveUpdate(_ body: () -> Void)
}

public extension AtomLoader {
func shouldUpdate(newValue: Value, oldValue: Value) -> Bool {
func shouldUpdateTransitively(newValue: Value, oldValue: Value) -> Bool {
true
}

func performUpdate(_ body: () -> Void) {
func performTransitiveUpdate(_ body: () -> Void) {
body()
}
}
Expand Down
10 changes: 5 additions & 5 deletions Sources/Atoms/Core/Loader/ModifiedAtomLoader.swift
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,13 @@ public struct ModifiedAtomLoader<Node: Atom, Modifier: AtomModifier>: AtomLoader

/// Returns a boolean value indicating whether it should notify updates downstream
/// by checking the equivalence of the given old value and new value.
public func shouldUpdate(newValue: Value, oldValue: Value) -> Bool {
modifier.shouldUpdate(newValue: newValue, oldValue: oldValue)
public func shouldUpdateTransitively(newValue: Value, oldValue: Value) -> Bool {
modifier.shouldUpdateTransitively(newValue: newValue, oldValue: oldValue)
}

/// Performs atom update.
public func performUpdate(_ body: () -> Void) {
modifier.performUpdate(body)
/// Performs transitive update for dependent atoms.
public func performTransitiveUpdate(_ body: () -> Void) {
modifier.performTransitiveUpdate(body)
}
}

Expand Down
128 changes: 94 additions & 34 deletions Sources/Atoms/Core/StoreContext.swift
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ internal struct StoreContext {
let key = AtomKey(atom, scopeKey: scopeKey)

if let cache = lookupCache(of: atom, for: key) {
update(atom: atom, for: key, newValue: value, cache: cache)
update(atom: atom, for: key, newValue: value, oldValue: cache.value)
}
}

Expand All @@ -95,7 +95,7 @@ internal struct StoreContext {

if let cache = lookupCache(of: atom, for: key) {
let newValue = mutating(cache.value, body)
update(atom: atom, for: key, newValue: newValue, cache: cache)
update(atom: atom, for: key, newValue: newValue, oldValue: cache.value)
}
}

Expand Down Expand Up @@ -127,9 +127,9 @@ internal struct StoreContext {
let scopeKey = lookupScopeKey(of: atom, isScopedOverriden: override?.isScoped ?? false)
let key = AtomKey(atom, scopeKey: scopeKey)
let cache = getCache(of: atom, for: key, override: override)
let isNewSubscription = subscriber.subscribingKeys.insert(key).inserted
let isNewSubscription = subscriber.subscribing.insert(key).inserted

store.state.subscriptions[key, default: [:]].updateValue(subscription, forKey: subscriber.key)
store.state.subscriptions[key, default: [:]][subscriber.key] = subscription
subscriber.unsubscribe = { keys in
unsubscribe(keys, for: subscriber.key)
}
Expand Down Expand Up @@ -164,7 +164,7 @@ internal struct StoreContext {

// Notify update unless it's cancelled or terminated by other operations.
if !Task.isCancelled && !context.isTerminated {
update(atom: atom, for: key, newValue: value, cache: cache)
update(atom: atom, for: key, newValue: value, oldValue: cache.value)
}

return value
Expand All @@ -186,7 +186,7 @@ internal struct StoreContext {

// Notify update unless it's cancelled or terminated by other operations.
if !Task.isCancelled && !transaction.isTerminated {
update(atom: atom, for: key, newValue: value, cache: cache)
update(atom: atom, for: key, newValue: value, oldValue: cache.value)
}

return value
Expand All @@ -201,7 +201,7 @@ internal struct StoreContext {

if let cache = lookupCache(of: atom, for: key) {
let newCache = makeCache(of: atom, for: key, override: override)
update(atom: atom, for: key, newValue: newCache.value, cache: cache)
update(atom: atom, for: key, newValue: newCache.value, oldValue: cache.value)
}
}

Expand Down Expand Up @@ -233,7 +233,7 @@ internal struct StoreContext {
let scopeKey = lookupScopeKey(of: atom, isScopedOverriden: override?.isScoped ?? false)
let key = AtomKey(atom, scopeKey: scopeKey)

subscriber.subscribingKeys.remove(key)
subscriber.subscribing.remove(key)
unsubscribe([key], for: subscriber.key)
}

Expand Down Expand Up @@ -327,7 +327,7 @@ private extension StoreContext {
coordinator: state.coordinator
) { newValue in
if let cache = lookupCache(of: atom, for: key) {
update(atom: atom, for: key, newValue: newValue, cache: cache)
update(atom: atom, for: key, newValue: newValue, oldValue: cache.value)
}
}
}
Expand All @@ -336,43 +336,103 @@ private extension StoreContext {
atom: Node,
for key: AtomKey,
newValue: Node.Loader.Value,
cache: AtomCache<Node>
oldValue: Node.Loader.Value
) {
let oldValue = cache.value
store.state.caches[key] = AtomCache(atom: atom, value: newValue)

store.state.caches[key] = mutating(cache) {
$0.value = newValue
// Check whether if the dependent atoms should be updated transitively.
guard atom._loader.shouldUpdateTransitively(newValue: newValue, oldValue: oldValue) else {
return
}

guard atom._loader.shouldUpdate(newValue: newValue, oldValue: oldValue) else {
return
// Perform side effects first.
let state = getState(of: atom, for: key)
let context = AtomCurrentContext(store: self, coordinator: state.coordinator)
atom.updated(newValue: newValue, oldValue: oldValue, context: context)
Comment on lines +348 to +351
Copy link
Contributor

Choose a reason for hiding this comment

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

Q: Does this mean we're changing the timing of the side effects, and in case usage accidentally relies on the current order (performing side effects at the last of this method), it might get affected?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yes, before the change, it runs a side effect after finishing updating downstream, but now it runs immediately after updating its value.
In the first place, the order in which the atoms were updated was not guaranteed, so it's not possible that they were used in a way that depended on the update order. (Even if there were, it wouldn't work properly)

Copy link
Contributor

Choose a reason for hiding this comment

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

In the first place, the order in which the atoms were updated was not guaranteed, so it's not possible that they were used in a way that depended on the update order. (Even if there were, it wouldn't work properly)

Understood, thank you!


// Calculate topological order for updating downstream efficiently.
let (edges, redundants) = topologicalSort(key: key, store: store)
var skippedDependencies = Set<AtomKey>()

// Updates the given atom.
func update(for key: AtomKey, cache: some AtomCacheProtocol) {
let override = lookupOverride(of: cache.atom)
let newCache = makeCache(of: cache.atom, for: key, override: override)

// Check whether if the dependent atoms should be updated transitively.
guard cache.atom._loader.shouldUpdateTransitively(newValue: newCache.value, oldValue: cache.value) else {
// Record the atom to avoid downstream from being update.
skippedDependencies.insert(key)
return
}

// Perform side effects before updating downstream.
let state = getState(of: cache.atom, for: key)
let context = AtomCurrentContext(store: self, coordinator: state.coordinator)
cache.atom.updated(newValue: newCache.value, oldValue: cache.value, context: context)
}

atom._loader.performUpdate {
// Notifies update to view subscriptions first.
if let subscriptions = store.state.subscriptions[key] {
for subscription in ContiguousArray(subscriptions.values) {
subscription.update()
}
// Performs update of the given atom with the dependency's context.
func performUpdate(for key: AtomKey, cache: some AtomCacheProtocol, dependency: some Atom) {
dependency._loader.performTransitiveUpdate {
update(for: key, cache: cache)
}
}

// Notifies update to downstream atoms.
if let children = store.graph.children[key] {
for child in ContiguousArray(children) {
// Reset the atom value and then notifies downstream atoms.
if let cache = store.state.caches[child] {
reset(cache.atom)
}
}
// Performs update of the given subscription with the dependency's context.
func performUpdate(subscription: Subscription, dependency: some Atom) {
dependency._loader.performTransitiveUpdate(subscription.update)
}

func validEdge(_ edge: Edge) -> Edge? {
// Do not transitively update atoms that have dependency recorded not to update downstream.
guard skippedDependencies.contains(edge.from) else {
return edge
}

// Notify value update to observers.
notifyUpdateToObservers()
// If the topological sorting has marked the vertex as a redundant, the update still performed.
guard let fromKey = redundants[edge.to]?.first(where: { !skippedDependencies.contains($0) }) else {
return nil
}

let state = getState(of: atom, for: key)
let context = AtomCurrentContext(store: self, coordinator: state.coordinator)
atom.updated(newValue: newValue, oldValue: oldValue, context: context)
// Convert edge's `from`, which represents a dependency atom, to a non-skipped one to
// change the update transaction context (e.g. animation).
return Edge(from: fromKey, to: edge.to)
}

// Perform transitive update for dependent atoms ahead of notifying updates to subscriptions.
for edge in edges {
switch edge.to {
case .atom(let key):
guard let edge = validEdge(edge) else {
// Record the atom to avoid downstream from being update.
skippedDependencies.insert(key)
continue
}

let cache = store.state.caches[key]
let dependencyCache = store.state.caches[edge.from]

if let cache, let dependencyCache {
performUpdate(for: key, cache: cache, dependency: dependencyCache.atom)
}

case .subscriber(let key):
guard let edge = validEdge(edge) else {
continue
}

let subscription = store.state.subscriptions[edge.from]?[key]
let dependencyCache = store.state.caches[edge.from]

if let subscription, let dependencyCache {
performUpdate(subscription: subscription, dependency: dependencyCache.atom)
}
}
}

// Notify the observers after all updates are completed.
notifyUpdateToObservers()
}

func unsubscribe<Keys: Sequence<AtomKey>>(_ keys: Keys, for subscriberKey: SubscriberKey) {
Expand Down
6 changes: 3 additions & 3 deletions Sources/Atoms/Core/Subscriber.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ internal struct Subscriber {
self.key = SubscriberKey(token: state.token)
}

var subscribingKeys: Set<AtomKey> {
get { state?.subscribingKeys ?? [] }
nonmutating set { state?.subscribingKeys = newValue }
var subscribing: Set<AtomKey> {
get { state?.subscribing ?? [] }
nonmutating set { state?.subscribing = newValue }
}

var unsubscribe: ((Set<AtomKey>) -> Void)? {
Expand Down
4 changes: 2 additions & 2 deletions Sources/Atoms/Core/SubscriberState.swift
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
@MainActor
final class SubscriberState {
let token = SubscriberKey.Token()
var subscribingKeys = Set<AtomKey>()
var subscribing = Set<AtomKey>()
var unsubscribe: ((Set<AtomKey>) -> Void)?

init() {}

deinit {
unsubscribe?(subscribingKeys)
unsubscribe?(subscribing)
}
}
72 changes: 72 additions & 0 deletions Sources/Atoms/Core/TopologicalSort.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
internal enum Vertex: Hashable {
case atom(key: AtomKey)
case subscriber(key: SubscriberKey)
}

internal struct Edge: Hashable {
let from: AtomKey
let to: Vertex
}

/// DFS topological sorting.
@MainActor
internal func topologicalSort(key: AtomKey, store: AtomStore) -> (
Copy link
Collaborator

@rasberik rasberik Apr 29, 2024

Choose a reason for hiding this comment

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

Global main actor func just to access main actor isolated store.graph, why not make it extension of AtomStore which already is on mainactor.. (not big fan of global functions with specific context)

[nit]
Also, do you think topologicalSort is a proper name given it returns edges? I can suggest to make it something more fitting the caller site → update(atom...) , e.g. childrenToUpdate() -> edges, redundants etc, and then hide the specific technique method like topologicallySorted() -> edges..

Copy link
Owner Author

Choose a reason for hiding this comment

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

Global main actor func just to access main actor isolated store.graph, why not make it extension of AtomStore which already is on mainactor.

Sounds good to me.

do you think topologicalSort is a proper name given it returns edges?

I believe how it is sorted is important for the caller as the additional algorithm on the StoreContext doesn't work except with topological sorting.
Also, any other naming wouldn't justify it returning the pair of edges and redundant.
More descriptive naming like topologicallySortedEdgesWithRedundantPath would make it clear but it sounds too much.

Copy link
Owner Author

Choose a reason for hiding this comment

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

I'm already on a bunch of refactoring in another branch, so will apply it there.

edges: ReversedCollection<[Edge]>,
redundants: [Vertex: [AtomKey]] // key = vertex, value = dependencies
) {
var trace = Set<Vertex>()
var edges = [Edge]()
var redundants = [Vertex: [AtomKey]]()

func traverse(key: AtomKey, isRedundant: Bool) {
if let children = store.graph.children[key] {
for child in ContiguousArray(children) {
traverse(key: child, from: key, isRedundant: isRedundant)
}
}

if let subscriptions = store.state.subscriptions[key] {
for subscriberKey in ContiguousArray(subscriptions.keys) {
traverse(key: subscriberKey, from: key, isRedundant: isRedundant)
}
}
}

func traverse(key: AtomKey, from fromKey: AtomKey, isRedundant: Bool) {
let vertex = Vertex.atom(key: key)
let isRedundant = isRedundant || trace.contains(vertex)

trace.insert(vertex)

// Do not stop traversing downstream even when edges are already traced
// to analyze the redundant edges later.
traverse(key: key, isRedundant: isRedundant)

if isRedundant {
redundants[vertex, default: []].append(fromKey)
}
else {
let edge = Edge(from: fromKey, to: vertex)
edges.append(edge)
}
}

func traverse(key: SubscriberKey, from fromKey: AtomKey, isRedundant: Bool) {
let vertex = Vertex.subscriber(key: key)
let isRedundant = isRedundant || trace.contains(vertex)

trace.insert(vertex)

if isRedundant {
redundants[vertex, default: []].append(fromKey)
}
else {
let edge = Edge(from: fromKey, to: vertex)
edges.append(edge)
}
}

traverse(key: key, isRedundant: false)

return (edges: edges.reversed(), redundants: redundants)
}
4 changes: 2 additions & 2 deletions Sources/Atoms/Modifier/AnimationModifier.swift
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ public struct AnimationModifier<T>: AtomModifier {
value
}

/// Performs atom update.
public func performUpdate(_ body: () -> Void) {
/// Performs transitive update for dependent atoms.
public func performTransitiveUpdate(_ body: () -> Void) {
withAnimation(animation, body)
}
}
Loading