clintropolis commented on PR #13698: URL: https://github.com/apache/druid/pull/13698#issuecomment-1403148309
Made an interesting find in this PR due to the travis consistently timing out during `TopNQueryRunnerTest`, which wasn't happening locally. Looking a bit closer though, I noticed that the test ran like 6 times slower when run as part of the entire `druid-processing` suite on my laptop: ``` $ mvn package -pl processing ... [INFO] Running org.apache.druid.query.topn.TopNQueryRunnerTest [INFO] Tests run: 82944, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 307.971 s - in org.apache.druid.query.topn.TopNQueryRunnerTest ... ``` compared to just running the specific test: ``` $ mvn package -pl processing -Dtest=org.apache.druid.query.topn.TopNQueryRunnerTest ... [INFO] Running org.apache.druid.query.topn.TopNQueryRunnerTest [INFO] Tests run: 82944, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 52.598 s - in org.apache.druid.query.topn.TopNQueryRunnerTest ... ``` Breaking out the profiler, and profiling both the individual test and the entire suite made me notice an interesting difference in the flame graph of all `TopNQueryRunnerTest` tests: <img width="1147" alt="Screenshot 2023-01-24 at 9 10 22 PM" src="https://user-images.githubusercontent.com/1577461/214492583-a5d57495-a489-47a4-a82e-222bc81b808c.png";> "What was `Mockito` doing in there? This test isn't using `Mockito` at all..." were my thoughts. Looking to usages of Mockito and `ByteBuffer`, the only thing using it was `NullColumnPartSerdeTest`. So, it turns out that a handful of `Mockito.mock(ByteBuffer.class)` calls in `NullColumnPartSerdeTest` was basically infecting all `ByteBuffer` usage in _all tests_ in `druid-processing` because of Mockito reasons. Modifying this test to use an empty but very regular heap `ByteBuffer` (since it didn't really need to mock anything, since the bytebuffer isn't called in this case) resulted in a dramatic speedup when run as part of the whole suite: ``` [INFO] Tests run: 82944, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 84.184 s - in org.apache.druid.query.topn.TopNQueryRunnerTest ``` So, moral of the story, it might be worth going over the tests and ensuring we limit usage of Mockito to uses that truly require it. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
![https://user-images.githubusercontent.com/1577461/214492583-a5d57495-a489-47a4-a82e-222bc81b808c.png"](https://user-images.githubusercontent.com/1577461/214492583-a5d57495-a489-47a4-a82e-222bc81b808c.png"){kind=link}