From cf70801a58cc2e588fecef10cf972b9a4b41e0b5 Mon Sep 17 00:00:00 2001 From: Anders Date: Wed, 23 Nov 2016 04:55:27 +0100 Subject: [PATCH 1/4] Lock-free disposal. --- Sources/Disposable.swift | 110 +++++++++++++++++++++++++++------------ 1 file changed, 77 insertions(+), 33 deletions(-) diff --git a/Sources/Disposable.swift b/Sources/Disposable.swift index 71962bd5b..3a7423559 100644 --- a/Sources/Disposable.swift +++ b/Sources/Disposable.swift @@ -6,16 +6,59 @@ // Copyright (c) 2014 GitHub. All rights reserved. // +#if os(macOS) || os(iOS) || os(tvOS) || os(watchOS) +import MachO +#endif + /// Represents something that can be “disposed”, usually associated with freeing /// resources or canceling work. public protocol Disposable: class { /// Whether this disposable has been disposed already. var isDisposed: Bool { get } - /// Method for disposing of resources when appropriate. + /// Disposing of the resources represented by `self`. If `self` has already + /// been disposed of, it does nothing. + /// + /// - note: Implementations must issue a memory barrier. func dispose() } +private struct DisposableState { +#if os(macOS) || os(iOS) || os(tvOS) || os(watchOS) + private var _isDisposed: Int32 + + fileprivate var isDisposed: Bool { + mutating get { + return OSAtomicCompareAndSwap32(1, 1, &_isDisposed) + } + } + + fileprivate init() { + _isDisposed = 0 + } + + fileprivate mutating func dispose() -> Bool { + return OSAtomicCompareAndSwap32Barrier(0, 1, &_isDisposed) + } +#else + private let _isDisposed: Atomic + + fileprivate var isDisposed: Bool { + mutating get { + return _isDisposed.value + } + } + + fileprivate init() { + _isDisposed = Atomic(false) + } + + fileprivate mutating func dispose() -> Bool { + return !_isDisposed.swap(true) + } +#endif +} + /// A type-erased disposable that forwards operations to an underlying disposable. public final class AnyDisposable: Disposable { private let disposable: Disposable @@ -36,25 +79,26 @@ public final class AnyDisposable: Disposable { /// A disposable that only flips `isDisposed` upon disposal, and performs no other /// work. public final class SimpleDisposable: Disposable { - private let _isDisposed = Atomic(false) + private var state = DisposableState() public var isDisposed: Bool { - return _isDisposed.value + return state.isDisposed } public init() {} public func dispose() { - _isDisposed.value = true + _ = state.dispose() } } /// A disposable that will run an action upon disposal. public final class ActionDisposable: Disposable { - private let action: Atomic<(() -> Void)?> + private var action: (() -> Void)? + private var state: DisposableState public var isDisposed: Bool { - return action.value == nil + return state.isDisposed } /// Initialize the disposable to run the given action upon disposal. @@ -62,18 +106,22 @@ public final class ActionDisposable: Disposable { /// - parameters: /// - action: A closure to run when calling `dispose()`. public init(action: @escaping () -> Void) { - self.action = Atomic(action) + self.action = action + self.state = DisposableState() } public func dispose() { - let oldAction = action.swap(nil) - oldAction?() + if state.dispose() { + action?() + action = nil + } } } /// A disposable that will dispose of any number of other disposables. public final class CompositeDisposable: Disposable { private let disposables: Atomic?> + private var state: DisposableState /// Represents a handle to a disposable previously added to a /// CompositeDisposable. @@ -106,7 +154,7 @@ public final class CompositeDisposable: Disposable { } public var isDisposed: Bool { - return disposables.value == nil + return state.isDisposed } /// Initialize a `CompositeDisposable` containing the given sequence of @@ -125,6 +173,7 @@ public final class CompositeDisposable: Disposable { } self.disposables = Atomic(bag) + self.state = DisposableState() } /// Initialize a `CompositeDisposable` containing the given sequence of @@ -145,9 +194,11 @@ public final class CompositeDisposable: Disposable { } public func dispose() { - if let ds = disposables.swap(nil) { - for d in ds.reversed() { - d.dispose() + if state.dispose() { + if let ds = disposables.swap(nil) { + for d in ds.reversed() { + d.dispose() + } } } } @@ -216,7 +267,7 @@ public final class ScopedDisposable: Disposable { } public func dispose() { - innerDisposable.dispose() + return innerDisposable.dispose() } } @@ -234,15 +285,11 @@ extension ScopedDisposable where InnerDisposable: AnyDisposable { /// A disposable that will optionally dispose of another disposable. public final class SerialDisposable: Disposable { - private struct State { - var innerDisposable: Disposable? = nil - var isDisposed = false - } - - private let state = Atomic(State()) + private let _innerDisposable: Atomic + private var state: DisposableState public var isDisposed: Bool { - return state.value.isDisposed + return state.isDisposed } /// The inner disposable to dispose of. @@ -251,18 +298,13 @@ public final class SerialDisposable: Disposable { /// disposable is automatically disposed. public var innerDisposable: Disposable? { get { - return state.value.innerDisposable + return _innerDisposable.value } set(d) { - let oldState: State = state.modify { state in - defer { state.innerDisposable = d } - return state - } - - oldState.innerDisposable?.dispose() - if oldState.isDisposed { - d?.dispose() + _innerDisposable.swap(d)?.dispose() + if let d = d, isDisposed { + d.dispose() } } } @@ -273,12 +315,14 @@ public final class SerialDisposable: Disposable { /// - parameters: /// - disposable: Optional disposable. public init(_ disposable: Disposable? = nil) { - innerDisposable = disposable + self._innerDisposable = Atomic(disposable) + self.state = DisposableState() } public func dispose() { - let orig = state.swap(State(innerDisposable: nil, isDisposed: true)) - orig.innerDisposable?.dispose() + if state.dispose() { + _innerDisposable.swap(nil)?.dispose() + } } } From 88cab2165b79e84d0e937d569d1eeaad5e605452 Mon Sep 17 00:00:00 2001 From: Anders Date: Sat, 26 Nov 2016 05:49:42 +0100 Subject: [PATCH 2/4] Introduce `UnsafeAtomicState` a simple lock-free FSM. --- Sources/Atomic.swift | 117 +++++++++++++++++++++++++++++++++++++++ Sources/Disposable.swift | 109 ++++++++++++++++++------------------ 2 files changed, 172 insertions(+), 54 deletions(-) diff --git a/Sources/Atomic.swift b/Sources/Atomic.swift index fbea4f8f6..51eade136 100644 --- a/Sources/Atomic.swift +++ b/Sources/Atomic.swift @@ -7,6 +7,123 @@ // import Foundation +#if os(macOS) || os(iOS) || os(tvOS) || os(watchOS) +import MachO +#endif + +/// Represents a finite state machine that can transit from one state to +/// another. +internal protocol AtomicStateProtocol { + associatedtype State: RawRepresentable + + /// Try to transit from the expected current state to the specified next + /// state. + /// + /// - parameters: + /// - expected: The expected state. + /// + /// - returns: + /// `true` if the transition succeeds. `false` otherwise. + mutating func tryTransiting(from expected: State, to next: State) -> Bool +} + +/// A simple, generic lock-free finite state machine. +/// +/// - warning: `deinitialize` must be called to dispose of the consumed memory. +internal struct UnsafeAtomicState: AtomicStateProtocol where State.RawValue == Int32 { + internal typealias Transition = (expected: State, next: State) +#if os(macOS) || os(iOS) || os(tvOS) || os(watchOS) + private let value: UnsafeMutablePointer + + /// Create a finite state machine with the specified initial state. + /// + /// - parameters: + /// - initial: The desired initial state. + internal init(_ initial: State) { + value = UnsafeMutablePointer.allocate(capacity: 1) + value.initialize(to: initial.rawValue) + } + + /// Deinitialize the finite state machine. + internal func deinitialize() { + value.deinitialize() + value.deallocate(capacity: 1) + } + + /// Compare the current state with the specified state. + /// + /// - parameters: + /// - expected: The expected state. + /// + /// - returns: + /// `true` if the current state matches the expected state. `false` + /// otherwise. + @inline(__always) + internal func `is`(_ expected: State) -> Bool { + return OSAtomicCompareAndSwap32Barrier(expected.rawValue, + expected.rawValue, + value) + } + + /// Try to transit from the expected current state to the specified next + /// state. + /// + /// - parameters: + /// - expected: The expected state. + /// + /// - returns: + /// `true` if the transition succeeds. `false` otherwise. + @inline(__always) + internal func tryTransiting(from expected: State, to next: State) -> Bool { + return OSAtomicCompareAndSwap32Barrier(expected.rawValue, + next.rawValue, + value) + } +#else + private let value: Atomic + + /// Create a finite state machine with the specified initial state. + /// + /// - parameters: + /// - initial: The desired initial state. + internal init(_ initial: State) { + value = Atomic(initial.rawValue) + } + + /// Deinitialize the finite state machine. + internal func deinitialize() {} + + /// Compare the current state with the specified state. + /// + /// - parameters: + /// - expected: The expected state. + /// + /// - returns: + /// `true` if the current state matches the expected state. `false` + /// otherwise. + internal func `is`(_ expected: State) -> Bool { + return value.modify { $0 == expected.rawValue } + } + + /// Try to transit from the expected current state to the specified next + /// state. + /// + /// - parameters: + /// - expected: The expected state. + /// + /// - returns: + /// `true` if the transition succeeds. `false` otherwise. + internal func tryTransiting(from expected: State, to next: State) -> Bool { + return value.modify { value in + if value == expected.rawValue { + value = next.rawValue + return true + } + return false + } + } +#endif +} final class PosixThreadMutex: NSLocking { private var mutex = pthread_mutex_t() diff --git a/Sources/Disposable.swift b/Sources/Disposable.swift index 3a7423559..c8f15f3df 100644 --- a/Sources/Disposable.swift +++ b/Sources/Disposable.swift @@ -6,10 +6,6 @@ // Copyright (c) 2014 GitHub. All rights reserved. // -#if os(macOS) || os(iOS) || os(tvOS) || os(watchOS) -import MachO -#endif - /// Represents something that can be “disposed”, usually associated with freeing /// resources or canceling work. public protocol Disposable: class { @@ -23,40 +19,24 @@ public protocol Disposable: class { func dispose() } -private struct DisposableState { -#if os(macOS) || os(iOS) || os(tvOS) || os(watchOS) - private var _isDisposed: Int32 - - fileprivate var isDisposed: Bool { - mutating get { - return OSAtomicCompareAndSwap32(1, 1, &_isDisposed) - } - } +/// Represents the state of a disposable. +private enum DisposableState: Int32 { + /// The disposable is active. + case active - fileprivate init() { - _isDisposed = 0 - } - - fileprivate mutating func dispose() -> Bool { - return OSAtomicCompareAndSwap32Barrier(0, 1, &_isDisposed) - } -#else - private let _isDisposed: Atomic - - fileprivate var isDisposed: Bool { - mutating get { - return _isDisposed.value - } - } - - fileprivate init() { - _isDisposed = Atomic(false) - } + /// The disposable has been disposed. + case disposed +} - fileprivate mutating func dispose() -> Bool { - return !_isDisposed.swap(true) +extension AtomicStateProtocol where State == DisposableState { + /// Try to transit from `active` to `disposed`. + /// + /// - returns: + /// `true` if the transition succeeds. `false` otherwise. + @inline(__always) + fileprivate mutating func tryDispose() -> Bool { + return tryTransiting(from: .active, to: .disposed) } -#endif } /// A type-erased disposable that forwards operations to an underlying disposable. @@ -79,26 +59,30 @@ public final class AnyDisposable: Disposable { /// A disposable that only flips `isDisposed` upon disposal, and performs no other /// work. public final class SimpleDisposable: Disposable { - private var state = DisposableState() + private var state = UnsafeAtomicState(DisposableState.active) public var isDisposed: Bool { - return state.isDisposed + return state.is(.disposed) } public init() {} public func dispose() { - _ = state.dispose() + _ = state.tryDispose() + } + + deinit { + state.deinitialize() } } /// A disposable that will run an action upon disposal. public final class ActionDisposable: Disposable { private var action: (() -> Void)? - private var state: DisposableState + private var state: UnsafeAtomicState public var isDisposed: Bool { - return state.isDisposed + return state.is(.disposed) } /// Initialize the disposable to run the given action upon disposal. @@ -107,36 +91,43 @@ public final class ActionDisposable: Disposable { /// - action: A closure to run when calling `dispose()`. public init(action: @escaping () -> Void) { self.action = action - self.state = DisposableState() + self.state = UnsafeAtomicState(DisposableState.active) } public func dispose() { - if state.dispose() { + if state.tryDispose() { action?() action = nil } } + + deinit { + state.deinitialize() + } } /// A disposable that will dispose of any number of other disposables. public final class CompositeDisposable: Disposable { private let disposables: Atomic?> - private var state: DisposableState + private var state: UnsafeAtomicState /// Represents a handle to a disposable previously added to a /// CompositeDisposable. public final class DisposableHandle { - private let bagToken: Atomic + private var state: UnsafeAtomicState + private var bagToken: RemovalToken? private weak var disposable: CompositeDisposable? fileprivate static let empty = DisposableHandle() fileprivate init() { - self.bagToken = Atomic(nil) + self.state = UnsafeAtomicState(.disposed) + self.bagToken = nil } fileprivate init(bagToken: RemovalToken, disposable: CompositeDisposable) { - self.bagToken = Atomic(bagToken) + self.state = UnsafeAtomicState(.active) + self.bagToken = bagToken self.disposable = disposable } @@ -145,16 +136,18 @@ public final class CompositeDisposable: Disposable { /// - note: This is useful to minimize memory growth, by removing /// disposables that are no longer needed. public func remove() { - if let token = bagToken.swap(nil) { + if state.tryDispose(), let token = bagToken { _ = disposable?.disposables.modify { $0?.remove(using: token) } + bagToken = nil + disposable = nil } } } public var isDisposed: Bool { - return state.isDisposed + return state.is(.disposed) } /// Initialize a `CompositeDisposable` containing the given sequence of @@ -173,7 +166,7 @@ public final class CompositeDisposable: Disposable { } self.disposables = Atomic(bag) - self.state = DisposableState() + self.state = UnsafeAtomicState(DisposableState.active) } /// Initialize a `CompositeDisposable` containing the given sequence of @@ -194,7 +187,7 @@ public final class CompositeDisposable: Disposable { } public func dispose() { - if state.dispose() { + if state.tryDispose() { if let ds = disposables.swap(nil) { for d in ds.reversed() { d.dispose() @@ -240,6 +233,10 @@ public final class CompositeDisposable: Disposable { public func add(_ action: @escaping () -> Void) -> DisposableHandle { return add(ActionDisposable(action: action)) } + + deinit { + state.deinitialize() + } } /// A disposable that, upon deinitialization, will automatically dispose of @@ -286,10 +283,10 @@ extension ScopedDisposable where InnerDisposable: AnyDisposable { /// A disposable that will optionally dispose of another disposable. public final class SerialDisposable: Disposable { private let _innerDisposable: Atomic - private var state: DisposableState + private var state: UnsafeAtomicState public var isDisposed: Bool { - return state.isDisposed + return state.is(.disposed) } /// The inner disposable to dispose of. @@ -316,14 +313,18 @@ public final class SerialDisposable: Disposable { /// - disposable: Optional disposable. public init(_ disposable: Disposable? = nil) { self._innerDisposable = Atomic(disposable) - self.state = DisposableState() + self.state = UnsafeAtomicState(DisposableState.active) } public func dispose() { - if state.dispose() { + if state.tryDispose() { _innerDisposable.swap(nil)?.dispose() } } + + deinit { + state.deinitialize() + } } /// Adds the right-hand-side disposable to the left-hand-side From 59dc77cdd39b6906b90df169a8b0627f4dfe22d3 Mon Sep 17 00:00:00 2001 From: Anders Date: Sat, 26 Nov 2016 13:25:00 +0100 Subject: [PATCH 3/4] Iterate the disposable bag directly in `CompositeDisposable`. `Bag` is unordered by contract, so there is no point to reverse it. --- Sources/Disposable.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/Disposable.swift b/Sources/Disposable.swift index c8f15f3df..468e05753 100644 --- a/Sources/Disposable.swift +++ b/Sources/Disposable.swift @@ -189,7 +189,7 @@ public final class CompositeDisposable: Disposable { public func dispose() { if state.tryDispose() { if let ds = disposables.swap(nil) { - for d in ds.reversed() { + for d in ds { d.dispose() } } From 0038c8bb70f5cb04e36aaf267af6f5a97e8d79c8 Mon Sep 17 00:00:00 2001 From: Anders Date: Tue, 29 Nov 2016 22:02:48 +0100 Subject: [PATCH 4/4] Drop the `mutating` modifier in `AtomicStateProtocol`. --- Sources/Atomic.swift | 2 +- Sources/Disposable.swift | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/Atomic.swift b/Sources/Atomic.swift index 51eade136..ddad85b01 100644 --- a/Sources/Atomic.swift +++ b/Sources/Atomic.swift @@ -24,7 +24,7 @@ internal protocol AtomicStateProtocol { /// /// - returns: /// `true` if the transition succeeds. `false` otherwise. - mutating func tryTransiting(from expected: State, to next: State) -> Bool + func tryTransiting(from expected: State, to next: State) -> Bool } /// A simple, generic lock-free finite state machine. diff --git a/Sources/Disposable.swift b/Sources/Disposable.swift index 468e05753..861682f53 100644 --- a/Sources/Disposable.swift +++ b/Sources/Disposable.swift @@ -34,7 +34,7 @@ extension AtomicStateProtocol where State == DisposableState { /// - returns: /// `true` if the transition succeeds. `false` otherwise. @inline(__always) - fileprivate mutating func tryDispose() -> Bool { + fileprivate func tryDispose() -> Bool { return tryTransiting(from: .active, to: .disposed) } }