davecom / SwiftGraph

A Graph Data Structure in Pure Swift
Apache License 2.0
758 stars 80 forks source link

Graph constructors #47

Closed ferranpujolcamins closed 6 years ago

ferranpujolcamins commented 6 years ago

Add performance test target running an optimized build and add performance tests for graph Constructors.

I'll be pushing our performance tests story in upcoming PRs

ferranpujolcamins commented 6 years ago

Right now the performance tests can't fail. They are only useful as an informative thing when you run them on your local machine. I need to investigate how they fit with CI.

Does it make sense to only run them in macOS?

davecom commented 6 years ago

I think it's okay for now if they only run in macOS. I was just looking and saw the Linux build failed, but I don't see an obvious reason why.

ferranpujolcamins commented 6 years ago

/home/travis/build/davecom/SwiftGraph/Tests/SwiftGraphPerformanceTests/SearchPerformanceTests.swift:25: Test Case 'SearchPerformanceTests.testDfsInStarGraph' measured [Time, seconds] average: 1.482, relative standard deviation: 11.300%, values: [1.388117, 1.383327, 1.406106, 1.376094, 1.376778, 1.390254, 1.393201, 1.535891, 1.797011, 1.777829], performanceMetricID:org.swift.XCTPerformanceMetric_WallClockTime, maxPercentRelativeStandardDeviation: 10.000%, maxStandardDeviation: 0.100

Just an statistical anomaly, a test rerun will probably pass. This is actually a reason to deactivate perf tests on linux: the results are not compared to any baseline, but if the variance is too high, the test fails, which is annoying.

I'm investigating how to set baselines in the travis machine for the macOS build, it's not easy :(

ferranpujolcamins commented 6 years ago

I asked on twitter, and the only way to set baselines on a CI server is with xcode server, so there's no way to set the performance tests baselines via xcodebuild. This means that we can't set baselines on Travis, so the performance tests assert nothing. So, we can do 4 things:

  1. Write a command-line tool that parses the output of xcodebuild (or swift test). 😞
  2. Use some swift library to manually measure the time of the code and get the result in a variable we can assert against (something like this or this) 😕
  3. Set up an xcode server 🙂 (but unless you have a spare mac...😕)
  4. Don't integrate performance tests on CI and manually assert them on each release / PR 😕

What are your thoughts?

davecom commented 6 years ago

I think we just leave the performance tests out for Linux. I think as long as we have all the "does this work" tests on Linux, we're good. Can we just leave the performance tests out of the static allTests variable for Linux?

ferranpujolcamins commented 6 years ago

Ok, I'll do that. So about Travis, I'll go with option 4 above and remove perf. tests from Travis until we have a solution from Apple.

ferranpujolcamins commented 6 years ago

Ready to merge. Sorry for going off the subject

davecom commented 6 years ago

Thank you!