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.

Sign up for GitHub

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

Jump to bottom

[CALCITE-3548] rename ubenchmark/src/main/jmh to ubenchmark/src/jmh/java #1617

Closed
wants to merge 1 commit into from
Closed

[CALCITE-3548] rename ubenchmark/src/main/jmh to ubenchmark/src/jmh/java #1617

wants to merge 1 commit into from

Conversation

amaliujia
Copy link
Contributor

@amaliujia amaliujia commented Nov 30, 2019 •
edited
Loading

src/jmh/java to make ./gradlew :ubenchmark:jmh find benchmark
classes to run.

Details in https://issues.apache.org/jira/browse/CALCITE-3548.

@amaliujia
[CALCITE-3548] rename ubenchmark/src/main/jmh to
60a5a03 
ubenchmark/src/jmh/java to make ./gradlew :ubenchmark:jmh find benchmark
classes to run.
@amaliujia
Copy link
Contributor Author

amaliujia commented Nov 30, 2019 •
edited
Loading

R: @vladimirdolzhenko

@amaliujia amaliujia changed the title [CALCITE-3548] rename ubenchmark/src/main/jmh to [CALCITE-3548]in ubenchmark module, rename/src/main/jmh to Nov 30, 2019
@amaliujia amaliujia closed this Nov 30, 2019
@amaliujia amaliujia reopened this Nov 30, 2019
@amaliujia
Copy link
Contributor Author

Actually seems like this change will make :ubenmark:jmh be triggered by ./gradlew check. Is the original structure intentional to avoid benmarks run by check?

If it is intentional, is there a separate command to run benchmarks?
If it is not intentional, do we want to avoid benchmarks run by check?

@danny0405
Copy link
Contributor

@vlsi Can you make some help ?

@vlsi
Copy link
Contributor

vlsi commented Nov 30, 2019

Actually seems like this change will make :ubenmark:jmh be triggered by ./gradlew check

It does not seem to be the case for me.

original structure intentional

I think the original structure is just a mistake. It was not intentional

@vlsi
Copy link
Contributor

vlsi commented Nov 30, 2019

Just in case: Gradle has --dry-run option (which has -m short form), and it does print the tasks to be executed.

$ ./gradlew :ubenchmark:check -m
...
:linq4j:compileJava SKIPPED
:core:compileKotlin SKIPPED
:core:fmppMain SKIPPED
:core:javaCCMain SKIPPED
:core:versionClass SKIPPED
:core:compileJava SKIPPED
:ubenchmark:compileJava SKIPPED
:ubenchmark:processResources SKIPPED
:ubenchmark:classes SKIPPED
:ubenchmark:compileTestJava SKIPPED
:ubenchmark:processTestResources SKIPPED
:ubenchmark:testClasses SKIPPED
:ubenchmark:compileJmhJava SKIPPED
:ubenchmark:processJmhResources SKIPPED
:ubenchmark:jmhClasses SKIPPED
:ubenchmark:checkstyleJmh SKIPPED
:ubenchmark:checkstyleMain SKIPPED
:ubenchmark:checkstyleTest SKIPPED
:ubenchmark:forbiddenApisJmh SKIPPED
:ubenchmark:forbiddenApisMain SKIPPED
:ubenchmark:forbiddenApisTest SKIPPED
:ubenchmark:forbiddenApis SKIPPED
:ubenchmark:spotlessJava SKIPPED
:ubenchmark:spotlessJavaCheck SKIPPED
:ubenchmark:spotlessKotlinGradle SKIPPED
:ubenchmark:spotlessKotlinGradleCheck SKIPPED
:ubenchmark:spotlessCheck SKIPPED
:ubenchmark:test SKIPPED
:ubenchmark:check SKIPPED

That is :jmh task is not going to be executed when check is requested.

@amaliujia
Copy link
Contributor Author

@vlsi thanks for the dry-run information (didn't know it but it's indeed handy). Maybe I have mis-read logs before.

Great then. This PR should make :ubenchmark:jmh work without affecting ubenchmark:check then.

@amaliujia amaliujia changed the title [CALCITE-3548]in ubenchmark module, rename/src/main/jmh to [CALCITE-3548]in ubenchmark module, rename src/main/jmh to Nov 30, 2019
@vlsi vlsi changed the title [CALCITE-3548]in ubenchmark module, rename src/main/jmh to [CALCITE-3548] rename ubenchmark/src/main/jmh to ubenchmark/src/jmh/java Dec 4, 2019
@vlsi vlsi closed this in ecc68ac Dec 4, 2019
@amaliujia amaliujia deleted the rw-fix_benchmark_jmh branch December 4, 2019 17:22
wangxlong pushed a commit to wangxlong/calcite that referenced this pull request Feb 13, 2020
@amaliujia @vlsi
[CALCITE-3548] unlock ./gradlew :ubenchmark:jmh to run benchmarks
9a55a97 
The proper location of the benchmark sources should be
ubenchmark/src/jmh/java
rather than
ubenchmark/src/main/jmh

Gradle convention is {module}/src/{sourceSetName}/{language}

fixes apache#1617
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@amaliujia @danny0405 @vlsi