Update Catch2 to version 2.13.10 #1566
Conversation
ashwinbhat
commented
Oct 12, 2023
•
ashwinbhat
commented
- Update Catch2 version to v2.13.10
- Catch2 v2.13.0 supports BENCHMARK tests.
- A new BENCHMARK test "GenShader: ShaderGen Performance" to benchmark document load, validation and shadergen times
Update to Catch v2.13.10 to use BENCHMARK tests.
Add a new "GenShader: ShaderGen Performance" test to measure time take to load, validate and generate shaders
|
This looks very promising, thanks @ashwinbhat! Looking at the build times, though, I see that our Dynamic Analysis tests take quite a while to complete the new benchmark step. Would it be possible to exclude the benchmark step from Dynamic Analysis builds, e.g. by setting a preprocessor flag when MATERIALX_DYNAMIC_ANALYSIS is enabled in a build, and checking for this before running this specific test? https://github.com/AcademySoftwareFoundation/MaterialX/blob/main/CMakeLists.txt#L56C8-L56C34 |
Add the Perfromance test only if MATERIALX_DYNAMIC_ANALYSIS is enabled.
Note that test implmentation is always compiled to catch any code/api changes to keep it updated, it is disabled from the default run.
Example output:
100% tests passed, 0 tests failed out of 72
Total Test time (real) = 121.03 sec
The following tests did not run:
51 - MaterialXTest_PerformanceTest_ShaderGen_and_Validation (Disabled)
|
Thanks for that update, @ashwinbhat, though I'm not sure this is having the desired effect! Looking at the builds in GitHub Actions, most are taking 25-30 minutes, but the two most recent builds from PR 1566 are taking more than 40 minutes, with the Dynamic Analysis build taking an unusually long time to complete (31 minutes, as compared to 8 minutes for other builds). One recommendation on the approach to controlling which tests run in Dynamic Analysis builds: What if we were to set a preprocessor flag named Here's an example of this pattern for setting preprocessor flags matching user options in the root CMake file: https://github.com/AcademySoftwareFoundation/MaterialX/blob/main/CMakeLists.txt#L168 |
Enable benchmarks only when MATERIALX_DYNAMIC_ANALYSIS is defined.
MATERIALX_DYNAMIC_ANALYSIS = OFF:
51: Test command: C:\source\MaterialX-adsk-fork-ext\build\bin\Release\MaterialXTest.exe "PerformanceTest: ShaderGen and Validation"
51: Working Directory: C:/source/MaterialX-adsk-fork-ext/build/bin
51: Test timeout computed to be: 10000000
51: Filters: PerformanceTest: ShaderGen and Validation
51: 0.000 s: PerformanceTest: ShaderGen and Validation
51: ===============================================================================
51: All tests passed (1 assertion in 1 test case)
With MATERIALX_DYNAMIC_ANALYSIS = ON:
test 51
Start 51: MaterialXTest_PerformanceTest_ShaderGen_and_Validation
51: Test command: C:\source\MaterialX-adsk-fork-ext\build\bin\Release\MaterialXTest.exe "PerformanceTest: ShaderGen and Validation"
51: Working Directory: C:/source/MaterialX-adsk-fork-ext/build/bin
51: Test timeout computed to be: 10000000
51: Filters: PerformanceTest: ShaderGen and Validation
51:
51: ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
51: MaterialXTest.exe is a Catch v2.13.10 host application.
51: Run with -? for options
51:
51: -------------------------------------------------------------------------------
51: PerformanceTest: ShaderGen and Validation
51: -------------------------------------------------------------------------------
51: C:\source\MaterialX-adsk-fork-ext\source\MaterialXTest\MaterialXGenGlsl\GenGlsl.cpp(123)
51: ...............................................................................
51:
51: benchmark name samples iterations estimated
51: mean low mean high mean
51: std dev low std dev high std dev
51: -------------------------------------------------------------------------------
51: Load documents, validate and
51: generate shader 100 1 2.34307 m
51: 1.38572 s 1.38087 s 1.39103 s
51: 25.8656 ms 23.34 ms 28.815 ms
51:
51:
51: 142.748 s: PerformanceTest: ShaderGen and Validation
51: ===============================================================================
51: All tests passed (8888 assertions in 1 test case)
51:
51/73 Test #51: MaterialXTest_PerformanceTest_ShaderGen_and_Validation ....... Passed 142.94 sec
…/MaterialX into bhata/updateCatch
|
Hi @jstone-lucasfilm I think I've updated it to follow you suggestion where PerformanceTest: ShaderGen and Validation is run only with MATERIALX_DYNAMIC_ANALYSIS = ON. I see that the last DYNAMIC_ANALYSIS build took 51m 19s. Should we turn it off completely if it's going to hurt PR time and developer productivity |
|
@ashwinbhat Oh, I may have miscommunicated my thinking here, and it's only when MATERIALX_DYNAMIC_ANALYSIS is off that we want to run these new benchmarking tests. In a dynamic analysis build, the compiler inserts additional error-checking instructions into the generated code, allowing us to enforce that conditions such as memory boundary overwrites never occur in our unit tests. But because these error-checking instructions dramatically slow down runtime performance, we want to omit our new benchmark tests when this mode has been enabled in the compiler, and we can check for this condition using the MATERIALX_DYNAMIC_ANALYSIS preprocessor flag that you've added. |
Adds a new benchmarking macro MATERIALX_BUILD_BENCHMARK_TESTS to enable GenShader Performance Tests using Catch2 Benchmarking system.
Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
jstone-lucasfilm
changed the title
jstone-lucasfilm
merged commit 926ac27
into
AcademySoftwareFoundation:main
