Test performance improvements

[arturbosch]

Findings:

• recent spek discovery improvements already help a bit (3s spek vs 1s jupiter -> 2s spek vs 1s jupiter for report-xml module)
• there are still some concurrent running things we don't understand
  • the more parallelization we use the more the test discovery rises
    • using a maximum of 4 workers help a lot
• using Gradle test fixtures improving the test discovery the most! Edit: not true, they start instantly just in IntelliJ
  • module report-txt without the detekt-test dependency starts instantly!!!
  • does this mean spek tries to discover to much? still running through all jars/the whole classpath??
  • have we setup the KtTestCompiler and ScriptEngine wrongly??
    • both classes are only used in methods and Kotlin objects guarantees us singletons
    • I've tried to remove the object declarations and using custom lazy inits -> no improvements

References:

• Closes The tests are slow #2902
• Remove verbose on cg scan and don't use MonoClock in jvm spekframework/spek#907
• [jvm] Scan classes in parallel spekframework/spek#905

[codecov]

Codecov Report

Merging #2921 into master will increase coverage by 0.01%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master    #2921      +/-   ##
============================================
+ Coverage     80.22%   80.23%   +0.01%     
+ Complexity     2449     2448       -1     
============================================
  Files           421      421              
  Lines          7408     7402       -6     
  Branches       1356     1352       -4     
============================================
- Hits           5943     5939       -4     
  Misses          764      764              
+ Partials        701      699       -2     

Impacted Files	Coverage Δ	Complexity Δ
...urbosch/detekt/rules/style/UselessCallOnNotNull.kt	80.95% <0.00%> (+3.17%)	7.00% <0.00%> (-1.00%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 05da2a7...12c704f. Read the comment docs.

[arturbosch]

Even with test fixtures the test discovery is noticeably slower when using --parallel (threads are starving)

[schalkms]

There are 2 hard problems in computer science: threads, naming things, and off-by-1 errors.

[arturbosch]

Did some profiling and spek is fast enough. Classgraph single-threaded works too.
I suspect 5 coroutine threads x #cores to be the reason (spek seems to be concurrent by default).
Junit Jupiter runs the tests sequentially (https://junit.org/junit5/docs/current/user-guide/#writing-tests-parallel-execution).
Which makes sense when using build tool parallelization.

[raniejade]

Hey! Out of curiosity, what does detekt-test do?

[raniejade]

One thing weird I found is when running detekt-core:test as itself, discovery is fast around 5 seconds, but when you run all the tests together - discovery goes up to 15s!

[raniejade]

Changing this made the discrepancy smaller from 5s (alone) -> 7s (all tests)

It was declared like this before:

        val finding = mockk<Finding>()
        every { finding.id }.returns("LongParameterList")
        every { finding.signature }.returns("Signature")

        val result = TestDetektion(finding)

So it was building the mock during discovery phase.

[raniejade]

One improvement I could suggest is avoid doing heavy stuff in the group scopes, use a combination of fixtures (beforeEachTest, beforeEachGroup, etc ..) and memoized to delay calculation values used during the execution phase.

[raniejade]

I've also made a change to setupKotlinEnvironment to delay calculation of the environment during the execution phase.

fun Root.setupKotlinEnvironment() {
    val wrapper by memoized(CachingMode.SCOPE, { createEnvironment() }, { it.dispose() } )

    @Suppress("UNUSED_VARIABLE") // name is used for delegation
    val env: KotlinCoreEnvironment by memoized(CachingMode.EACH_GROUP) { wrapper.env }
}

[raniejade]

After a couple of testing from this branch, the biggest difference I can see is when making sure nothing heavy is calculated in the discovery phase. I've tried making cg scanning and execution sequential but the difference are negligible 1-2 seconds.

[1] I made changes to your test (2m 9.625s): https://scans.gradle.com/s/hvit4fbjj54j6/tests?collapse-all

vs

[2] No changes (2m 37.258s): https://scans.gradle.com/s/k3cwnp74ersju/tests?collapse-all

From what I can see the individual test timings in Gradle's scan doesn't account for discovery time - but just execution time.

[arturbosch]

@raniejade thank you very much for taking the time to help us out!
I've incorporated your suggestions.

I also removed the coroutine usage but the time between starting the test task and executing the tests is nearly the same (1-2s as you mentioned).

So the only thing we can do is to stop introducing new modules which need the kotlin compiler or mockk :/.

[raniejade]

So the only thing we can do is to stop introducing new modules which need the kotlin compiler or mockk :/.

I think if we can just make sure that no heavy initialization happens in the discovery phase, will help improve discovery. There are probably other areas in your tests that are doing some initialization in the discovery phase, fixing them might help improve test speeds.

[schalkms]

Awesome stuff!!

[arturbosch]

So the only thing we can do is to stop introducing new modules which need the kotlin compiler or mockk :/.

I think if we can just make sure that no heavy initialization happens in the discovery phase, will help improve discovery. There are probably other areas in your tests that are doing some initialization in the discovery phase, fixing them might help improve test speeds.

Yes, it is definitely the way to move heavy setup code from discovery to the execution phase.
However in my build scans I couldn't see any performance improvements from this.
~20-30seconds fluctuations can be normal for a fresh and a hot gradle daemon (+ building buildSrc module).
Reducing the parallelization was the only factor which helped in incremental build time.

Edit: modules like tooling report-html and rules have no setup code in the tests but the test discovery increases from 200ms to like 5s just by adding more gradle workers ...