-
Notifications
You must be signed in to change notification settings - Fork 263
[SR-1355] Add Performance Measurement APIs #109
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
Conversation
… metric * Failures can be triggered by a sufficiently high STDDEV value on a set of measurements * There is no support for saving baselines or failing due to regressions
@swift-ci please test |
Amazing!! Thanks for this. Swift 3.0, here we come!!
Is this something you think we should address? Are there consequences for end-users if we split it up, such as requiring them to link multiple modules, besides XCTest?
This has been in the back of my mind for a while now: Apple places its asynchronous testing APIs in a category on More granular comments to follow. |
func recordMeasurements(results: String, file: StaticString, line: UInt) | ||
func recordFailure(description: String, file: StaticString, line: UInt) | ||
func recordAPIViolation(description: String, file: StaticString, line: UInt) | ||
} |
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.
The awesome documentation for PerformanceMetric
spoiled me, now I wish there was documentation here, too 😝
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.
Good call!
This looks fantastic! I only had minor nit-picks, which I feel could adequately be addressed after merging. Considering:
I'd encourage you to hit the big green merge button. 😄 I don't know much about time measurement, but as for the other things you mention in your pull request:
|
The time measurement parts sound reasonable to me for an initial implementation:
|
Thanks for the positive feedback guys! 😎 I'm going to go ahead and merge this. Exciting that it appears to be making it in before the creation of the |
Oh, regarding modules - I do believe it should be possible to split the library internals into multiple modules without require consumers to modify their imports (which would make this a non-starter), but I'm not certain yet. @ddunbar or @mxcl could likely confirm the feasibility of this given their experience with SwiftPM? |
What's in this pull request
This is an initial implementation of the XCTest performance measurement APIs. All public facing APIs from Apple XCTest are present, but not all behaviors are implemented at this point. It is currently possible to measure the wall clock time that a given block of code takes to execute, and see the measured times, the mean value, and the relative standard deviation in the printed test output.
The core behaviors mimic those described in the Apple XCTest framework headers, supplemented by information presented in the Testing in Xcode 6 WWDC session.
This does not include any support for recording baseline values or triggering test failures due to a regression relative to a baseline.
Resolves SR-1355.
Code structure
Although the build system does not currently facilitate the use of multiple Swift modules, I have structured the code with modularity in mind. The
PerformanceMeter.swift
andWallClockTimeMetric.swift
contain the heart of the implementation and have (almost) no dependence on other parts of the XCTest library.XCTestCase+Performance.swift
contains the public APIs which delegate to thePerformanceMeter
class.Test coverage
There are extensive functional tests covering of API misuse scenarios and basic measurement output, however many finer points (e.g. standard deviation calculation and situations triggering failure) have only cursory tests due to the lack of a usable unit testing infrastructure (see this PR for previous discussion about this.)
I attempted to make the tests robust, however due to the time-sensitive nature of the APIs under test, there is an increased possibility of flakiness that could appear when running in a CI environment. I would greatly appreciate feedback on this aspect of the test cases and what approaches we could take to minimize the risk of false negative results. I would also like to hear feedback on the
test_measuresWallClockTimeInBlock
test case inPerformance/main.swift
, which currently includes a 1-second sleep in order totest that time passing is being measured.
Error reporting
I took pains to implement failure reporting due to API misuse, just as Apple XCTest does. One element of this is reporting the file and line number where the misuse occurred. I have taken cues from @modocache's
XCTestExpectation
implementation and addedfile
andline
params where necessary to capture that information. The current implementation records test failures for API misuse, allowing test suite execution to continue at the cost of some additional bookkeeping code. An alternative would be to simplyfatalError
in these scenarios, since they are clearly programmer error. I would appreciate others' input on these options.Time measurement
I am no expert in benchmarking, so I am not especially confident in the optimal mechanism for measuring the passage of time for this purpose, however after some cursory research, I settled on using
NSProcessInfo
'ssystemUptime
property, which ultimately calls down tomach_absolute_time
on Darwin, andclock_gettime
withCLOCK_MONOTONIC
on Linux. As far as I can tell, these are the appropriate primitives to use for this purpose in these environments. One potential source of error is introduced in that the time values are being converted toDouble
, but I'm unsure of the practical impact of that.While not complete, this is still a significant step towards feature parity with Apple XCTest, and should be sufficient to start gathering feedback on future implementation directions. In particular, it should put us in a good position to productively carry on discussions like this one from @drewcrawford which have great potential to improve the value that these tools provide.
cc @ddunbar @paulofaria