From 7787ad6ce4e404dfcb8f7894b3fb41dbdcacbcc4 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Mon, 12 Aug 2024 11:28:06 -0400 Subject: [PATCH 1/5] Reduce overhead of `.expectationChecked` event handling in `#expect()`. This PR refactors the implementation of `#expect()` and `#require()` a bit such that they don't incur more than minimal overhead posting `.expectationChecked` events if nobody is listening for them (which, currently, nobody is.) We considered removing `.expectationChecked` outright, but XCTest has historically had a number of requests for a way to observe calls to `XCTAssert()` etc. even when they pass, so we opted not to remove the event kind at this time. This PR also introduces a cache for fully-qualified type names so that we don't need to call into the runtime to get them as often. Overall speedup is approximately **90% or 11x**. Test time for a tight loop of 1,000,000 `#expect()` calls goes from 5.898893 seconds down to 0.515558291 seconds (as measured on my work computer.) Resolves rdar://133517028. --- Package.swift | 1 + .../ExpectationChecking+Macro.swift | 6 ++-- .../Testing/Parameterization/TypeInfo.swift | 11 ++++++ .../Testing/Running/Runner.RuntimeState.swift | 34 +++++++++++++++++-- Tests/TestingTests/MiscellaneousTests.swift | 11 ++++++ 5 files changed, 58 insertions(+), 5 deletions(-) diff --git a/Package.swift b/Package.swift index 4d7891274..5bf6c100d 100644 --- a/Package.swift +++ b/Package.swift @@ -148,6 +148,7 @@ extension Array where Element == PackageDescription.SwiftSetting { .enableExperimentalFeature("AvailabilityMacro=_clockAPI:macOS 13.0, iOS 16.0, watchOS 9.0, tvOS 16.0"), .enableExperimentalFeature("AvailabilityMacro=_regexAPI:macOS 13.0, iOS 16.0, tvOS 16.0, watchOS 9.0"), .enableExperimentalFeature("AvailabilityMacro=_swiftVersionAPI:macOS 13.0, iOS 16.0, tvOS 16.0, watchOS 9.0"), + .enableExperimentalFeature("AvailabilityMacro=_synchronizationAPI:macOS 15.0, iOS 18.0, watchOS 11.0, tvOS 18.0, visionOS 2.0"), .enableExperimentalFeature("AvailabilityMacro=_distantFuture:macOS 99.0, iOS 99.0, watchOS 99.0, tvOS 99.0, visionOS 99.0"), ] diff --git a/Sources/Testing/Expectations/ExpectationChecking+Macro.swift b/Sources/Testing/Expectations/ExpectationChecking+Macro.swift index 2a3e137a3..33d68a255 100644 --- a/Sources/Testing/Expectations/ExpectationChecking+Macro.swift +++ b/Sources/Testing/Expectations/ExpectationChecking+Macro.swift @@ -95,8 +95,10 @@ public func __checkValue( // Post an event for the expectation regardless of whether or not it passed. // If the current event handler is not configured to handle events of this // kind, this event is discarded. - var expectation = Expectation(evaluatedExpression: expression, isPassing: condition, isRequired: isRequired, sourceLocation: sourceLocation) - Event.post(.expectationChecked(expectation)) + lazy var expectation = Expectation(evaluatedExpression: expression, isPassing: condition, isRequired: isRequired, sourceLocation: sourceLocation) + if Configuration.deliverExpectationCheckedEventsAnywhere { + Event.post(.expectationChecked(expectation)) + } // Early exit if the expectation passed. if condition { diff --git a/Sources/Testing/Parameterization/TypeInfo.swift b/Sources/Testing/Parameterization/TypeInfo.swift index 671674531..ccf0cd824 100644 --- a/Sources/Testing/Parameterization/TypeInfo.swift +++ b/Sources/Testing/Parameterization/TypeInfo.swift @@ -74,6 +74,9 @@ public struct TypeInfo: Sendable { // MARK: - Name extension TypeInfo { + /// An in-memory cache of fully-qualified type name components. + private static let _fullyQualifiedNameComponentsCache = Locked<[ObjectIdentifier: [String]]>() + /// The complete name of this type, with the names of all referenced types /// fully-qualified by their module names when possible. /// @@ -92,6 +95,10 @@ extension TypeInfo { public var fullyQualifiedNameComponents: [String] { switch _kind { case let .type(type): + if let cachedResult = Self._fullyQualifiedNameComponentsCache.rawValue[ObjectIdentifier(type)] { + return cachedResult + } + var result = String(reflecting: type) .split(separator: ".") .map(String.init) @@ -109,6 +116,10 @@ extension TypeInfo { // those out as they're uninteresting to us. result = result.filter { !$0.starts(with: "(unknown context at") } + Self._fullyQualifiedNameComponentsCache.withLock { fullyQualifiedNameComponentsCache in + fullyQualifiedNameComponentsCache[ObjectIdentifier(type)] = result + } + return result case let .nameOnly(fullyQualifiedNameComponents, _, _): return fullyQualifiedNameComponents diff --git a/Sources/Testing/Running/Runner.RuntimeState.swift b/Sources/Testing/Running/Runner.RuntimeState.swift index 3928a5e6b..1ef098750 100644 --- a/Sources/Testing/Running/Runner.RuntimeState.swift +++ b/Sources/Testing/Running/Runner.RuntimeState.swift @@ -8,6 +8,8 @@ // See https://swift.org/CONTRIBUTORS.txt for Swift project authors // +private import Synchronization + extension Runner { /// A type which collects the task-scoped runtime state for a running /// ``Runner`` instance, the tests it runs, and other objects it interacts @@ -111,7 +113,10 @@ extension Configuration { /// - Returns: A unique number identifying `self` that can be /// passed to `_removeFromAll(identifiedBy:)`` to unregister it. private func _addToAll() -> UInt64 { - Self._all.withLock { all in + if deliverExpectationCheckedEvents, #available(_synchronizationAPI, *) { + Self._deliverExpectationCheckedEventsAnywhereCount.add(1, ordering: .sequentiallyConsistent) + } + return Self._all.withLock { all in let id = all.nextID all.nextID += 1 all.instances[id] = self @@ -126,11 +131,34 @@ extension Configuration { /// `_addToAll()`. If `nil`, this function has no effect. private func _removeFromAll(identifiedBy id: UInt64?) { if let id { - Self._all.withLock { all in - _ = all.instances.removeValue(forKey: id) + let configuration = Self._all.withLock { all in + all.instances.removeValue(forKey: id) + } + if let configuration, configuration.deliverExpectationCheckedEvents, #available(_synchronizationAPI, *) { + Self._deliverExpectationCheckedEventsAnywhereCount.subtract(1, ordering: .sequentiallyConsistent) } } } + + /// An atomic counter that tracks the number of "current" configurations that + /// have set ``deliverExpectationCheckedEvents`` to `true`. + /// + /// On older Apple platforms, this property is not available and ``all`` is + /// directly consulted instead (which is less efficient.) + @available(_synchronizationAPI, *) + private static let _deliverExpectationCheckedEventsAnywhereCount = Atomic(0) + + /// Whether or not events of the kind + /// ``Event/Kind-swift.enum/expectationChecked(_:)`` should be delivered to + /// the event handler of _any_ configuration set as current for a task in the + /// current process. + static var deliverExpectationCheckedEventsAnywhere: Bool { + if #available(_synchronizationAPI, *) { + Self._deliverExpectationCheckedEventsAnywhereCount.load(ordering: .sequentiallyConsistent) > 0 + } else { + Self.all.contains(where: \.deliverExpectationCheckedEvents) + } + } } // MARK: - Current test and test case diff --git a/Tests/TestingTests/MiscellaneousTests.swift b/Tests/TestingTests/MiscellaneousTests.swift index f77b698d6..3cd900eee 100644 --- a/Tests/TestingTests/MiscellaneousTests.swift +++ b/Tests/TestingTests/MiscellaneousTests.swift @@ -532,4 +532,15 @@ struct MiscellaneousTests { failureBreakpoint() #expect(failureBreakpointValue == 1) } + + @available(_clockAPI, *) + @Test("Repeated calls to #expect() run in reasonable time", .disabled("time-sensitive")) + func repeatedlyExpect() { + let duration = Test.Clock().measure { + for _ in 0 ..< 1_000_000 { + #expect(true as Bool) + } + } + #expect(duration < .seconds(1)) + } } From 81d2ed6f0ecc39ffd01a7c953e14182b6fa53500 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Mon, 12 Aug 2024 13:46:34 -0400 Subject: [PATCH 2/5] Name tweak --- .../Expectations/ExpectationChecking+Macro.swift | 2 +- .../Testing/Running/Runner.RuntimeState.swift | 16 ++++++++++------ 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/Sources/Testing/Expectations/ExpectationChecking+Macro.swift b/Sources/Testing/Expectations/ExpectationChecking+Macro.swift index 33d68a255..ab0caa9e5 100644 --- a/Sources/Testing/Expectations/ExpectationChecking+Macro.swift +++ b/Sources/Testing/Expectations/ExpectationChecking+Macro.swift @@ -96,7 +96,7 @@ public func __checkValue( // If the current event handler is not configured to handle events of this // kind, this event is discarded. lazy var expectation = Expectation(evaluatedExpression: expression, isPassing: condition, isRequired: isRequired, sourceLocation: sourceLocation) - if Configuration.deliverExpectationCheckedEventsAnywhere { + if Configuration.deliverExpectationCheckedEvents { Event.post(.expectationChecked(expectation)) } diff --git a/Sources/Testing/Running/Runner.RuntimeState.swift b/Sources/Testing/Running/Runner.RuntimeState.swift index 1ef098750..dcae6d017 100644 --- a/Sources/Testing/Running/Runner.RuntimeState.swift +++ b/Sources/Testing/Running/Runner.RuntimeState.swift @@ -114,7 +114,7 @@ extension Configuration { /// passed to `_removeFromAll(identifiedBy:)`` to unregister it. private func _addToAll() -> UInt64 { if deliverExpectationCheckedEvents, #available(_synchronizationAPI, *) { - Self._deliverExpectationCheckedEventsAnywhereCount.add(1, ordering: .sequentiallyConsistent) + Self._deliverExpectationCheckedEventsCount.add(1, ordering: .sequentiallyConsistent) } return Self._all.withLock { all in let id = all.nextID @@ -135,7 +135,7 @@ extension Configuration { all.instances.removeValue(forKey: id) } if let configuration, configuration.deliverExpectationCheckedEvents, #available(_synchronizationAPI, *) { - Self._deliverExpectationCheckedEventsAnywhereCount.subtract(1, ordering: .sequentiallyConsistent) + Self._deliverExpectationCheckedEventsCount.subtract(1, ordering: .sequentiallyConsistent) } } } @@ -146,17 +146,21 @@ extension Configuration { /// On older Apple platforms, this property is not available and ``all`` is /// directly consulted instead (which is less efficient.) @available(_synchronizationAPI, *) - private static let _deliverExpectationCheckedEventsAnywhereCount = Atomic(0) + private static let _deliverExpectationCheckedEventsCount = Atomic(0) /// Whether or not events of the kind /// ``Event/Kind-swift.enum/expectationChecked(_:)`` should be delivered to /// the event handler of _any_ configuration set as current for a task in the /// current process. - static var deliverExpectationCheckedEventsAnywhere: Bool { + /// + /// To determine if an individual instance of ``Configuration`` is listening + /// for these events, consult the per-instance + /// ``Configuration/deliverExpectationCheckedEvents`` property. + static var deliverExpectationCheckedEvents: Bool { if #available(_synchronizationAPI, *) { - Self._deliverExpectationCheckedEventsAnywhereCount.load(ordering: .sequentiallyConsistent) > 0 + _deliverExpectationCheckedEventsCount.load(ordering: .sequentiallyConsistent) > 0 } else { - Self.all.contains(where: \.deliverExpectationCheckedEvents) + all.contains(where: \.deliverExpectationCheckedEvents) } } } From fec47a411b3753b54ee71d22d1c19aca771460b4 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Mon, 12 Aug 2024 13:55:18 -0400 Subject: [PATCH 3/5] _removeFromAll does not need an optional argument --- Sources/Testing/Running/Runner.RuntimeState.swift | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/Sources/Testing/Running/Runner.RuntimeState.swift b/Sources/Testing/Running/Runner.RuntimeState.swift index dcae6d017..7f76a93ef 100644 --- a/Sources/Testing/Running/Runner.RuntimeState.swift +++ b/Sources/Testing/Running/Runner.RuntimeState.swift @@ -129,14 +129,12 @@ extension Configuration { /// - Parameters: /// - id: The unique identifier of this instance, as previously returned by /// `_addToAll()`. If `nil`, this function has no effect. - private func _removeFromAll(identifiedBy id: UInt64?) { - if let id { - let configuration = Self._all.withLock { all in - all.instances.removeValue(forKey: id) - } - if let configuration, configuration.deliverExpectationCheckedEvents, #available(_synchronizationAPI, *) { - Self._deliverExpectationCheckedEventsCount.subtract(1, ordering: .sequentiallyConsistent) - } + private func _removeFromAll(identifiedBy id: UInt64) { + let configuration = Self._all.withLock { all in + all.instances.removeValue(forKey: id) + } + if let configuration, configuration.deliverExpectationCheckedEvents, #available(_synchronizationAPI, *) { + Self._deliverExpectationCheckedEventsCount.subtract(1, ordering: .sequentiallyConsistent) } } From e316fe5794642ca99a3a30623eca3b86f4f6eeeb Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Mon, 12 Aug 2024 13:55:33 -0400 Subject: [PATCH 4/5] Fix comment --- Sources/Testing/Running/Runner.RuntimeState.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/Testing/Running/Runner.RuntimeState.swift b/Sources/Testing/Running/Runner.RuntimeState.swift index 7f76a93ef..898dfa068 100644 --- a/Sources/Testing/Running/Runner.RuntimeState.swift +++ b/Sources/Testing/Running/Runner.RuntimeState.swift @@ -128,7 +128,7 @@ extension Configuration { /// /// - Parameters: /// - id: The unique identifier of this instance, as previously returned by - /// `_addToAll()`. If `nil`, this function has no effect. + /// `_addToAll()`. private func _removeFromAll(identifiedBy id: UInt64) { let configuration = Self._all.withLock { all in all.instances.removeValue(forKey: id) From 369950312e5e3724eae6af6583b4062c9e92c366 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Mon, 12 Aug 2024 14:12:55 -0400 Subject: [PATCH 5/5] Infer atomic generic type --- Sources/Testing/Running/Runner.RuntimeState.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/Testing/Running/Runner.RuntimeState.swift b/Sources/Testing/Running/Runner.RuntimeState.swift index 898dfa068..34ad152c5 100644 --- a/Sources/Testing/Running/Runner.RuntimeState.swift +++ b/Sources/Testing/Running/Runner.RuntimeState.swift @@ -144,7 +144,7 @@ extension Configuration { /// On older Apple platforms, this property is not available and ``all`` is /// directly consulted instead (which is less efficient.) @available(_synchronizationAPI, *) - private static let _deliverExpectationCheckedEventsCount = Atomic(0) + private static let _deliverExpectationCheckedEventsCount = Atomic(0) /// Whether or not events of the kind /// ``Event/Kind-swift.enum/expectationChecked(_:)`` should be delivered to