Skip to content
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

Performance failure mode #10061

Merged
merged 7 commits into from
Oct 17, 2019

Conversation

mjkkirschner
Copy link
Member

@mjkkirschner mjkkirschner commented Oct 14, 2019

Purpose

still a work in progress

Adds an additional check to the comparison command of the DynamoPerformanceTest CLI -

  • Compare command will now also return a Pass/Fail state
  • This state is logged to the console as part of the comparison results
  • this state is determined by a function - the default function just checks the increase in mean time is not > 10% - the ability to pass different functions for different tests in the future is possible.
  • I have temporarily removed all but one performance graph just to speed up my tests while developing this and related CI changes.
  • If any test is marked as FAIL - then the CLI will exit with a non zero error code and log a message to the console.

TODO:

  • verify that non zero exit code will mark the jenkins build a failure
  • fix log file - result is not serialized there for some reason even though it shows in console log. rather it is just on the wrong line.

see related changes:
https://git.autodesk.com/Dynamo/DynamoPerformance-CI/pull/8

Declarations

Check these if you believe they are true

  • The code base is in a better state after this PR
  • Is documented according to the standards
  • The level of testing this PR includes is appropriate
  • User facing strings, if any, are extracted into *.resx files
  • All tests pass using the self-service CI.
  • Snapshot of UI changes, if any.
  • Changes to the API follow Semantic Versioning, and are documented in the API Changes document.

Reviewers

@QilongTang
@reddyashish

add log text for result
add comparisonData property to each Comparer and use it
@mjkkirschner
Copy link
Member Author

 Comparison of Model tests: 
12:10:31  
12:10:31  | Method |   Graph | Version |        Mean |    Error |   StdDev | Result |
12:10:31  |--------|---------|---------|-------------|----------|----------|--------|
12:10:31  |   Open | Aniform |    Base |   126.83 ms |  2.07 ms |  0.54 ms |
12:10:31  |        |         |     New |   134.50 ms |  2.59 ms |  0.67 ms |
12:10:31  |        |         |         |      +6.05% |   25.28% |   25.28% |PASS
12:10:31  |    Run | Aniform |    Base | 3,779.43 ms | 82.77 ms | 49.26 ms |
12:10:31  |        |         |     New | 3,678.10 ms | 66.58 ms | 39.62 ms |
12:10:31  |        |         |         |      -2.68% |  -19.56% |  -19.56% |PASS
12:10:31  
12:10:31  Comparison of View tests: 
12:10:31  
12:10:31  exception while comparing results System.IO.FileNotFoundException: Could not find file 'C:\Jenkins\workspace\Dynamo\DynamoPerformance-CI\dyn1884\baselines\DynamoPerformanceTests.DynamoViewPerformanceTestBase-report.csv'.
12:10:31     at Microsoft.VisualBasic.FileIO.TextFieldParser.ValidatePath(String path)
12:10:31     at Microsoft.VisualBasic.FileIO.TextFieldParser.InitializeFromPath(String path, Encoding defaultEncoding, Boolean detectEncoding)
12:10:31     at DynamoPerformanceTests.ResultsComparer.ImportResultsCSV(String csvPath) in c:\DynamoPerformance-CI\Dynamo\tools\Performance\DynamoPerformanceTests\ResultsComparer.cs:line 329
12:10:31     at DynamoPerformanceTests.ResultsComparer.CreateComparisons(String basePath, String newPath) in c:\DynamoPerformance-CI\Dynamo\tools\Performance\DynamoPerformanceTests\ResultsComparer.cs:line 270
12:10:31     at DynamoPerformanceTests.Program.Compare(String baseResultsPath, String newResultsPath, String savePath) in c:\DynamoPerformance-CI\Dynamo\tools\Performance\DynamoPerformanceTests\PerformanceTestConsoleApp.cs:line 143 
12:10:31   Could not find file 'C:\Jenkins\workspace\Dynamo\DynamoPerformance-CI\dyn1884\baselines\DynamoPerformanceTests.DynamoViewPerformanceTestBase-report.csv'.
12:10:31  

@mjkkirschner mjkkirschner added PTAL Please Take A Look 👀 and removed WIP labels Oct 15, 2019
@mjkkirschner mjkkirschner changed the title [WIP] Performance failure mode Performance failure mode Oct 15, 2019
Copy link
Contributor

@QilongTang QilongTang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with a few comments

@reddyashish
Copy link
Contributor

Looks good to me.

@mjkkirschner
Copy link
Member Author

mjkkirschner commented Oct 16, 2019

@QilongTang addressed your comments, I'm having trouble getting to build so that I can check the failure - I'm going to close the PR and reopen after I am done testing. - whoops, need to close PR on the CI side - sorry.

PTAL.

@mjkkirschner mjkkirschner reopened this Oct 16, 2019
@QilongTang QilongTang added LGTM Looks good to me and removed PTAL Please Take A Look 👀 labels Oct 16, 2019
@mjkkirschner
Copy link
Member Author

when theres a failure we'll see something like this:
@aparajit-pratap @smangarole @QilongTang

20:24:03  |--------|---------|---------|-------------|-----------|----------|--------|
20:24:03  |   Open | Aniform |    Base |   126.83 ms |   2.07 ms |  0.54 ms |
20:24:03  |        |         |     New |   123.60 ms |   1.09 ms |  0.39 ms |
20:24:03  |        |         |         |      -2.55% |    -47.3% |  -27.63% |PASS
20:24:03  |    Run | Aniform |    Base |   779.43 ms |  82.77 ms | 49.26 ms |
20:24:03  |        |         |     New | 3,797.70 ms | 152.90 ms | 90.99 ms |
20:24:03  |        |         |         |    +387.24% |    84.73% |   84.73% |FAIL
20:24:03  
20:24:03  Comparison of View tests: 
20:24:03  
20:24:03  exception while comparing results System.IO.FileNotFoundException: Could not find file 'C:\Jenkins\workspace\Dynamo\DynamoPerformance-CI\dyn1884\baselines\DynamoPerformanceTests.DynamoViewPerformanceTestBase-report.csv'.
snip
  Comparison failed, some model performance benchmarks failed. Please see log above for details.
20:24:03  localBuildContainer
20:24:03  failed during test stage
20:24:03  + throw "failed during test stage"

@mjkkirschner mjkkirschner merged commit a97eeee into DynamoDS:master Oct 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LGTM Looks good to me
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants