New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
measureRunTime use DispatchTime #2545
Conversation
Motivation: Using `Date()` in benchmarks leaves us open to the possibility that a clock update could occur between the start and end times affecting the measured interval. Modifications: Switch `measureRunTime` and related functions to use `DispatchTime` uptime in nanoseconds to calculate intervals Result: Tests should no longer be susceptible to clock updates.
@@ -37,7 +37,7 @@ public func measureRunTime(_ body: () throws -> Int) rethrows -> TimeInterval { | |||
|
|||
public func measureRunTimeAndPrint(desc: String, body: () throws -> Int) rethrows -> Void { | |||
print("measuring: \(desc)") | |||
print("\(try measureRunTime(body))s") | |||
print("\(try measureRunTime(body)/1_000_000)s") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably do floating point arithmetic here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, TimeInterval
does.
@@ -119,7 +119,8 @@ func runSystemCallWrapperPerformanceTest(testAssertFunction: (@autoclosure () -> | |||
precondition(isDebugMode) | |||
print("WARNING: Syscall wrapper test: Over 100% overhead allowed. Running in debug assert configuration which allows \(allowedOverheadPercent)% overhead :(. Consider running in Release mode.") | |||
} | |||
testAssertFunction(directCallTime * (1.0 + Double(allowedOverheadPercent)/100) > withSystemCallWrappersTime, | |||
let upper = Double(directCallTime) * (1.0 + Double(allowedOverheadPercent)/100) | |||
testAssertFunction( upper > Double(withSystemCallWrappersTime), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
testAssertFunction( upper > Double(withSystemCallWrappersTime), | |
testAssertFunction(upper > Double(withSystemCallWrappersTime), |
public func measureRunTime(_ body: () throws -> Int) rethrows -> TimeInterval { | ||
func measureOne(_ body: () throws -> Int) rethrows -> TimeInterval { | ||
let start = Date() | ||
public func measureRunTime(_ body: () throws -> Int) rethrows -> UInt64 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will probably make your life easier to keep the function signature the same, and just compute the TimeInterval
internal to measureOne
.
Motivation:
Using
Date()
in benchmarks leaves us open to the possibility that a clock update could occur between the start and end times affecting the measured interval.Modifications:
Switch
measureRunTime
and related functions to useDispatchTime
uptime in nanoseconds to calculate intervalsResult:
Tests should no longer be susceptible to clock updates.