add MicroBenchMark model#2851
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #2851 +/- ##
============================================
- Coverage 37.19% 37.15% -0.04%
+ Complexity 2559 2556 -3
============================================
Files 435 435
Lines 20020 20020
Branches 2426 2426
============================================
- Hits 7447 7439 -8
- Misses 11907 11912 +5
- Partials 666 669 +3
Continue to review full report at Codecov.
|
davidzollo
left a comment
There was a problem hiding this comment.
+1
good job，some tests need to be done
kezhenxu94
left a comment
There was a problem hiding this comment.
It's not rational to run the micro-benchmark in the test, this would drastically slow down the CI builds, suggestion is excluding the micro-benchmark from the test phase
I have excluded it, please check |
You just exclude the sonar check |
Sorry, I don’t quite understand what you mean, because it does not contain the test module itself, can you please elaborate |
dolphinscheduler-microbench/pom.xml
Outdated
| <plugin> | ||
| <groupId>org.apache.maven.plugins</groupId> | ||
| <artifactId>maven-shade-plugin</artifactId> | ||
| <version>3.1.1</version> | ||
| <executions> | ||
| <execution> | ||
| <phase>package</phase> | ||
| <goals> | ||
| <goal>shade</goal> |
There was a problem hiding this comment.
Why we need to shade these classes?
There was a problem hiding this comment.
Why we need to shade these classes?
I think microbench can be used as a runnable package, do you have any good suggestions?
There was a problem hiding this comment.
Why we need to shade these classes?
I think microbench can be used as a runnable package, do you have any good suggestions?
Interesting :) In my opinion, shading classes is to avoid version/naming conflicts between a library and the user's dependencies, but for micro-benchmark tests, we only have one version and the users don't interact with this module, am I right?
JMH can be packaged into a uber jar and executable indeed, but the classes are all known to us, there is no class that will conflict with the existed ones, so no shading is necessary, right? If I'm wrong, please correct me 😄
There was a problem hiding this comment.
Why we need to shade these classes?
I think microbench can be used as a runnable package, do you have any good suggestions?
Interesting :) In my opinion, shading classes is to avoid version/naming conflicts between a library and the user's dependencies, but for micro-benchmark tests, we only have one version and the users don't interact with this module, am I right?
JMH can be packaged into a uber jar and executable indeed, but the classes are all known to us, there is no class that will conflict with the existed ones, so no shading is necessary, right? If I'm wrong, please correct me 😄
My original intention is to use it as a data reference for the optimization of our code performance. In addition, I hope that after the performance improvement, users of dolphinscheduler can use this to verify.So i do this， What do you think?
There was a problem hiding this comment.
Why we need to shade these classes?
I think microbench can be used as a runnable package, do you have any good suggestions?
Interesting :) In my opinion, shading classes is to avoid version/naming conflicts between a library and the user's dependencies, but for micro-benchmark tests, we only have one version and the users don't interact with this module, am I right?
JMH can be packaged into a uber jar and executable indeed, but the classes are all known to us, there is no class that will conflict with the existed ones, so no shading is necessary, right? If I'm wrong, please correct me 😄
Thank you for your suggestion, I have completed the change, please review it
Sorry I missed that, please ignore me |
|
is there a document about how to use it ? |
Sorry, I will add related documents on the website later |
|
Look good to me once the related doc is added :) |
|
Kudos, SonarCloud Quality Gate passed!
|
|
the how to use doc ： apache/dolphinscheduler-website#125 |
|
good job， looking forward for next contribution |
Tips
What is the purpose of the pull request
add jmh model,we can have better data support for code optimization or code performance. Later, we can also expand on this basis to facilitate users to do performance tests.
Brief change log
Verify this pull request
This pull request is code cleanup without any test coverage.
Do I need to write a test case?
This change added tests and can be verified as follows:
-- Manually verified the change by testing locally