Add Timer.measure methods
#140
Conversation
|
@swift-server-bot test this please |
|
@swift-server-bot add to allowlist |
Sources/Metrics/Metrics.swift
Outdated
| return try body() | ||
| } | ||
|
|
||
| /// Convenience for measuring duration of a closure with a provided clock. |
There was a problem hiding this comment.
Nit: the doc for this method and the non-async version don't match. Maybe remove the last bit to match that one, since we're defaulting the clock param anyways?
| /// Convenience for measuring duration of a closure with a provided clock. | |
| /// Convenience for measuring duration of a closure. |
|
@swift-server-bot test this please |
FranzBusch
force-pushed
the
fb-timer-measure
branch
2 times, most recently
from
b541236 to
8cc9d38
Compare
# Motivation This PR supersedes #135. The goal is to make it easier to measure asynchronous code when using `Metrics`. # Modification This PR does: - Deprecate the current static method for measuring synchronous code - Add a new instance method to measure synchronous code - Add a new instance method to measure asynchronous code # Result It is now easier to measure asynchronous code.
FranzBusch
force-pushed
the
fb-timer-measure
branch
from
8cc9d38 to
199b458
Compare
|
@fabianfett @simonjbeaumont @czechboy0 I dusted off this PR again and brought up to the latest Swift standards. If you could take another look that would be great. |
| @@ -24,6 +24,9 @@ extension Timer { | |||
| /// - label: The label for the Timer. | |||
| /// - dimensions: The dimensions for the Timer. | |||
| /// - body: Closure to run & record. | |||
| #if compiler(>=6.0) | |||
There was a problem hiding this comment.
Do we think it's okay to have a deprecation based on a compiler version? What's the experience for an adopter who supports the same 3 Swift versions as us, do they also have to provide two solutions?
Also, the new API has an OS requirement, so we should make sure we don't put any adopters into a situation where they can't fix this warning without bumping their OS requirement (which today requires a major bump).
| /// - clock: The clock used for measuring the duration. Defaults to the continuous clock. | ||
| /// - body: The closure to record the duration of. | ||
| @inlinable | ||
| @available(macOS 13, iOS 16, tvOS 16, watchOS 9, *) |
There was a problem hiding this comment.
What's the recommendation for adopters who aren't yet on these OS versions as deployment targets?
Should we consider offering a variant if this method without the bumped OS requirements, which use pre-Clock way to measure time, for packages with older OS deployment targets? Or do we just recommend they write their own?
Motivation
This PR supersedes #135. The goal is to make it easier to measure asynchronous code when using
Metrics.Modification
This PR does: