From 5887703da1167f6a046c9968844c801d315b270c Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Wed, 4 Sep 2024 12:54:15 -0400 Subject: [PATCH] [6.0] Reduce overhead of `.expectationChecked` event handling in `#expect()` (take 2) (#659) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **Explanation:** Optimizes the implementation of `#expect()`, in particular the parts that ask for fully-qualified type names and generate `.expectationChecked` events. **Scope:** 6.0 branch **Issue:** N/A **Original PR:** https://github.com/swiftlang/swift-testing/pull/610, also includes fixup commit 6ba948ac9894a72576b03800998e7e47ebe48ae7 **Risk:** Moderate—refactors code inside `#expect()` and introduces a new lock and atomic value used by them. **Testing:** New unit test coverage, existing coverage. **Reviewer:** @briancroom @suzannaratcliff --- Package.swift | 1 + .../ExpectationChecking+Macro.swift | 6 ++- .../Testing/Parameterization/TypeInfo.swift | 11 +++++ .../Testing/Running/Runner.RuntimeState.swift | 44 ++++++++++++++++--- Tests/TestingTests/MiscellaneousTests.swift | 11 +++++ .../shared/AvailabilityDefinitions.cmake | 1 + 6 files changed, 65 insertions(+), 9 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 02a3ccd72..badf41599 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.deliverExpectationCheckedEvents { + 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..34ad152c5 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._deliverExpectationCheckedEventsCount.add(1, ordering: .sequentiallyConsistent) + } + return Self._all.withLock { all in let id = all.nextID all.nextID += 1 all.instances[id] = self @@ -123,12 +128,37 @@ 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 { - Self._all.withLock { all in - _ = all.instances.removeValue(forKey: id) - } + /// `_addToAll()`. + 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) + } + } + + /// 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 _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. + /// + /// 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, *) { + _deliverExpectationCheckedEventsCount.load(ordering: .sequentiallyConsistent) > 0 + } else { + all.contains(where: \.deliverExpectationCheckedEvents) } } } 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)) + } } diff --git a/cmake/modules/shared/AvailabilityDefinitions.cmake b/cmake/modules/shared/AvailabilityDefinitions.cmake index e7595f223..24e186aef 100644 --- a/cmake/modules/shared/AvailabilityDefinitions.cmake +++ b/cmake/modules/shared/AvailabilityDefinitions.cmake @@ -13,4 +13,5 @@ add_compile_options( "SHELL:$<$:-Xfrontend -define-availability -Xfrontend \"_clockAPI:macOS 13.0, iOS 16.0, watchOS 9.0, tvOS 16.0\">" "SHELL:$<$:-Xfrontend -define-availability -Xfrontend \"_regexAPI:macOS 13.0, iOS 16.0, tvOS 16.0, watchOS 9.0\">" "SHELL:$<$:-Xfrontend -define-availability -Xfrontend \"_swiftVersionAPI:macOS 13.0, iOS 16.0, tvOS 16.0, watchOS 9.0\">" + "SHELL:$<$:-Xfrontend -define-availability -Xfrontend \"_synchronizationAPI:macOS 15.0, iOS 18.0, watchOS 11.0, tvOS 18.0, visionOS 2.0\">" "SHELL:$<$:-Xfrontend -define-availability -Xfrontend \"_distantFuture:macOS 99.0, iOS 99.0, watchOS 99.0, tvOS 99.0, visionOS 99.0\">")