You are viewing a plain text version of this content. The canonical link for it is here.

Posted to issues@commons.apache.org by C0rWin <gi...@git.apache.org> on 2016/08/13 00:36:37 UTC

[GitHub] commons-lang pull request #182: Add maven dependency for JMH framework.

GitHub user C0rWin opened a pull request:

    https://github.com/apache/commons-lang/pull/182

    Add maven dependency for JMH framework.

    In order to provide patch for LANG-1110, required dependcy on JMH lib.
    Current commit add benchmark profile and ability to run JMH based benchmark by
    executing "mvn test -P benchmark" command, moreover it's also possible to
    specify exact benchmark name by running "mvn test -P benchmark
    -Dbenchmark=benchmark.full.class.name".

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/C0rWin/commons-lang LANG-1256

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/commons-lang/pull/182.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #182
    
----
commit 9f4c0ec4197331687e410aca492d4071eded2462
Author: Artem Barger <ar...@bargr.net>
Date:   2016-08-13T00:33:08Z

    Add maven dependency for JMH framework.
    
    In order to provide patch for LANG-1110, required dependcy on JMH lib.
    Current commit add benchmark profile and ability to run JMH based benchmark by
    executing "mvn test -P benchmark" command, moreover it's also possible to
    specify exact benchmark name by running "mvn test -P benchmark
    -Dbenchmark=benchmark.full.class.name".

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] commons-lang issue #182: Add maven dependency for JMH framework.

Posted by C0rWin <gi...@git.apache.org>.

Github user C0rWin commented on the issue:

    https://github.com/apache/commons-lang/pull/182
  
    Hi Bruno,
    
    
    On Sun, Feb 12, 2017 at 6:35 AM, Bruno P. Kinoshita <
    notifications@github.com> wrote:
    
    > Hi @C0rWin <https://github.com/C0rWin>
    >
    > Some time ago I found this pull request and had to learn what was JMH :-)
    > added a comment on LANG-1110
    > <https://issues.apache.org/jira/browse/LANG-1110>, and today had time to
    > revisit the issue and take a look at the replies.
    >
    > So Commons CSV
    > <https://github.com/apache/commons-csv/blob/a7757849d3e9b2a37967ce208ab22f6d14567c98/pom.xml#L389>
    > and Commons RNG
    > <https://github.com/apache/commons-rng/tree/688f3611131e05a072d998edb93e13492c3ed256/commons-rng-jmh>
    > use JMH too. Both projects apply a similar approach, with the main
    > difference being that Commons RNG - maybe for being a multi-module project
    > - contains a jmh Maven module.
    >
    > However, the dependencies and plug-in execution are part of a benchmark
    > profile. Following the suggestions in LANG-1110
    > <https://issues.apache.org/jira/browse/LANG-1110>, would be nice if we:
    >
    >    - Used a similar approach, such as profile, class names, etc
    >    - Update jmh dependencies to the latest
    >    <http://search.maven.org/#search%7Cga%7C1%7Cjmh-core>
    >
    > I wanted to check with you before I opened a separate pull request. Would
    > you be willing to update this pull request again with these changes? If so,
    > just ping me and I'll reply here as soon as possible. Have plenty of time
    > to work on Open Source projects in the next two months :-)
    >
    
    I can update my pull request to align it with according to your request.
    
    Thanks,
        Artem Barger.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] commons-lang issue #182: Add maven dependency for JMH framework.

Posted by PascalSchumacher <gi...@git.apache.org>.

Github user PascalSchumacher commented on the issue:

    https://github.com/apache/commons-lang/pull/182
  
    Thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] commons-lang issue #182: Add maven dependency for JMH framework.

Posted by coveralls <gi...@git.apache.org>.

Github user coveralls commented on the issue:

    https://github.com/apache/commons-lang/pull/182
  
    
    [![Coverage Status](https://coveralls.io/builds/7432683/badge)](https://coveralls.io/builds/7432683)
    
    Coverage decreased (-0.02%) to 93.413% when pulling **9f4c0ec4197331687e410aca492d4071eded2462 on C0rWin:LANG-1256** into **648eebba22d6398a01b0924975bedced8469ac80 on apache:master**.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] commons-lang issue #182: Add maven dependency for JMH framework.

Posted by C0rWin <gi...@git.apache.org>.

Github user C0rWin commented on the issue:

    https://github.com/apache/commons-lang/pull/182
  
    @PascalSchumacher As far as I know coverage could be reduced only if I've added new code which is not tested or not covered by existing tests.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] commons-lang issue #182: Add maven dependency for JMH framework.

Posted by PascalSchumacher <gi...@git.apache.org>.

Github user PascalSchumacher commented on the issue:

    https://github.com/apache/commons-lang/pull/182
  
    @C0rWin The OpenJDK 7 Build did not finish: https://travis-ci.org/apache/commons-lang/jobs/151969189 maybe that caused the change. Or there is a random element in the existing test?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] commons-lang issue #182: Add maven dependency for JMH framework.

Posted by kinow <gi...@git.apache.org>.

Github user kinow commented on the issue:

    https://github.com/apache/commons-lang/pull/182
  
    @C0rWin what about trying to add the test from [LANG-1110](https://issues.apache.org/jira/browse/LANG-1110) to the pull request? Look at the attachments section.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] commons-lang issue #182: Add maven dependency for JMH framework.

Posted by C0rWin <gi...@git.apache.org>.

Github user C0rWin commented on the issue:

    https://github.com/apache/commons-lang/pull/182
  
    Actually once this one will be in, I have a few tests based on JMH which I can push.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] commons-lang issue #182: Add maven dependency for JMH framework.

Posted by C0rWin <gi...@git.apache.org>.

Github user C0rWin commented on the issue:

    https://github.com/apache/commons-lang/pull/182
  
    No Java code was involved in this PR, how that possible coverage reduced?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] commons-lang issue #182: Add maven dependency for JMH framework.

Posted by C0rWin <gi...@git.apache.org>.

Github user C0rWin commented on the issue:

    https://github.com/apache/commons-lang/pull/182
  
    I'd guess it fails since there is no JMH tests available in the project, so it fails to execute them.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] commons-lang issue #182: Add maven dependency for JMH framework.

Posted by kinow <gi...@git.apache.org>.

Github user kinow commented on the issue:

    https://github.com/apache/commons-lang/pull/182
  
    Oh, missed that one, will take a look today to compare with [csv].
    
    I've never seen it failing in Jenkins or Travis CI, but I suspect it is run manually, whenever someone needs to work on the classes involved in the benchmark tests for new features or debugging issues in different environments.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] commons-lang issue #182: Add maven dependency for JMH framework.

Posted by kinow <gi...@git.apache.org>.

Github user kinow commented on the issue:

    https://github.com/apache/commons-lang/pull/182
  
    Tried running the following to test the pull request:
    
    ```
    $ mvn clean install
    $ mvn test -Pbenchmark -e -X
    ```
    
    And it kept failing with:
    
    ```
    [INFO] BUILD FAILURE
    [INFO] ------------------------------------------------------------------------
    [INFO] Total time: 9.718 s
    [INFO] Finished at: 2017-02-22T20:00:57+13:00
    [INFO] Final Memory: 32M/486M
    [INFO] ------------------------------------------------------------------------
    [ERROR] Failed to execute goal org.codehaus.mojo:exec-maven-plugin:1.5.0:exec (benchmark) on project commons-lang3: Command execution failed. Process exited with an error: 1 (Exit value: 1) -> [Help 1]
    org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.codehaus.mojo:exec-maven-plugin:1.5.0:exec (benchmark) on project commons-lang3: Command execution failed.
    	at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:212)
    	at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:153)
    	at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:145)
    	at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject(LifecycleModuleBuilder.java:116)
    	at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject(LifecycleModuleBuilder.java:80)
    	at org.apache.maven.lifecycle.internal.builder.singlethreaded.SingleThreadedBuilder.build(SingleThreadedBuilder.java:51)
    	at org.apache.maven.lifecycle.internal.LifecycleStarter.execute(LifecycleStarter.java:128)
    	at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:307)
    	at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:193)
    	at org.apache.maven.DefaultMaven.execute(DefaultMaven.java:106)
    	at org.apache.maven.cli.MavenCli.execute(MavenCli.java:863)
    	at org.apache.maven.cli.MavenCli.doMain(MavenCli.java:288)
    	at org.apache.maven.cli.MavenCli.main(MavenCli.java:199)
    	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    	at java.lang.reflect.Method.invoke(Method.java:498)
    	at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced(Launcher.java:289)
    	at org.codehaus.plexus.classworlds.launcher.Launcher.launch(Launcher.java:229)
    	at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode(Launcher.java:415)
    	at org.codehaus.plexus.classworlds.launcher.Launcher.main(Launcher.java:356)
    Caused by: org.apache.maven.plugin.MojoExecutionException: Command execution failed.
    	at org.codehaus.mojo.exec.ExecMojo.execute(ExecMojo.java:302)
    	at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo(DefaultBuildPluginManager.java:134)
    	at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:207)
    	... 20 more
    Caused by: org.apache.commons.exec.ExecuteException: Process exited with an error: 1 (Exit value: 1)
    	at org.apache.commons.exec.DefaultExecutor.executeInternal(DefaultExecutor.java:404)
    	at org.apache.commons.exec.DefaultExecutor.execute(DefaultExecutor.java:166)
    	at org.codehaus.mojo.exec.ExecMojo.executeCommandLine(ExecMojo.java:764)
    	at org.codehaus.mojo.exec.ExecMojo.executeCommandLine(ExecMojo.java:711)
    	at org.codehaus.mojo.exec.ExecMojo.execute(ExecMojo.java:289)
    	... 22 more
    ```
    
    Tried playing with dependency versions and settings, using Commons CSV as example (which works for me, as long as manually install one maven dependency as per pom.xml comment) with no luck.
    
    Will give it another try tomorrow at work.
    
    ```
    $ mvn -v
    Apache Maven 3.3.9 (bb52d8502b132ec0a5a3f4c09453c07478323dc5; 2015-11-11T05:41:47+13:00)
    Maven home: /opt/maven
    Java version: 1.8.0_121, vendor: Oracle Corporation
    Java home: /usr/lib/jvm/java-8-oracle/jre
    Default locale: en_US, platform encoding: UTF-8
    OS name: "linux", version: "4.4.0-62-generic", arch: "amd64", family: "unix"
    ```



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] commons-lang issue #182: Add maven dependency for JMH framework.

Posted by kinow <gi...@git.apache.org>.

Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/182

Hi @C0rWin

Some time ago I found this pull request and had to learn what was JMH :-) added a comment on [LANG-1110](https://issues.apache.org/jira/browse/LANG-1110), and today had time to revisit the issue and take a look at the replies.

So [Commons CSV](https://github.com/apache/commons-csv/blob/a7757849d3e9b2a37967ce208ab22f6d14567c98/pom.xml#L389) and [Commons RNG](https://github.com/apache/commons-rng/tree/688f3611131e05a072d998edb93e13492c3ed256/commons-rng-jmh) use JMH too. Both projects apply a similar approach, with the main difference being that Commons RNG - maybe for being a multi-module project - contains a jmh Maven module.

However, the dependencies and plug-in execution are part of a benchmark profile. Following the suggestions in [LANG-1110](https://issues.apache.org/jira/browse/LANG-1110), would be nice if we:

* Used a similar approach, such as profile, class names, etc
* Update [jmh dependencies to the latest](http://search.maven.org/#search%7Cga%7C1%7Cjmh-core)

I wanted to check with you before I opened a separate pull request. Would you be willing to update this pull request again with these changes? If so, just ping me and I'll reply here as soon as possible. Have plenty of time to work on Open Source projects in the next two months :-)

Cheers
Bruno

---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] commons-lang issue #182: Add maven dependency for JMH framework.

Posted by PascalSchumacher <gi...@git.apache.org>.

Github user PascalSchumacher commented on the issue:

    https://github.com/apache/commons-lang/pull/182
  
    I guess this is ready to merge?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] commons-lang issue #182: Add maven dependency for JMH framework.

Posted by PascalSchumacher <gi...@git.apache.org>.

Github user PascalSchumacher commented on the issue:

    https://github.com/apache/commons-lang/pull/182
  
    As far as I can tell @C0rWin updated the pull request 9 days ago (after your conversation).
    
    By the way merging this only makes sense if commons lang actually uses JHM. Do you (or somebody else) plan to work on this?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] commons-lang issue #182: Add maven dependency for JMH framework.

Posted by coveralls <gi...@git.apache.org>.

Github user coveralls commented on the issue:

    https://github.com/apache/commons-lang/pull/182
  
    
    [![Coverage Status](https://coveralls.io/builds/10113778/badge)](https://coveralls.io/builds/10113778)
    
    Coverage remained the same at 94.536% when pulling **d38aea4c5b82f0bdd4800fa4153a55ba6e8fa4b9 on C0rWin:LANG-1256** into **b715d18f09591c510dded3a447a3ad1aacfa2173 on apache:master**.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---