[JENKINS-57653] Introduce JMH benchmarks to Jenkins Test Harness #135

AbhyudayaSharma · 2019-06-03T12:25:26Z

Introduces Java Microbenchmark Harness benchmarks to the test harness to allow JMH benchmarks to be run from everywhere with Jenkins instance available in JMH forks.

This was first implemented in the Role Strategy Plugin. Demo from the May 31st meeting: https://www.youtube.com/watch?v=sr28UADG1AE

Part of Google Summer of Code 2019.

Downstream PRs:

[JENKINS-57425] Add profile to run benchmarks only when the benchmark property is set plugin-pom#206
[INFRA-2118] Add function to run JMH benchmarks jenkins-infra/pipeline-library#91
[JENKINS-57928] Allow configuring JMH benchmarks using CASC configuration-as-code-plugin#921

@jenkinsci/gsoc2019-role-strategy

oleg-nenashev · 2019-06-03T12:35:50Z

pom.xml

@@ -85,8 +85,7 @@ THE SOFTWARE.
    <dependency>
      <groupId>org.jenkins-ci.main</groupId>
      <artifactId>jenkins-war</artifactId>
-      <!--to have access to User.getById-->
-      <version>1.651.2</version>
+      <version>2.60.3</version>


As discussed in Gitter, I am about submitting a wider thread about dropping Java 7 support in our devtools

Started https://groups.google.com/forum/#!topic/jenkinsci-dev/SdMOMKs7XIQ

pom.xml

src/main/java/jenkins/jmh/JmhBenchmarkState.java

pom.xml

AbhyudayaSharma · 2019-06-04T05:21:46Z

I can confirm the same test failure happens by upgrading Jenkins core on the latest master.

oleg-nenashev · 2019-06-04T05:27:22Z

My bad about the matrix auth suggestion. Let's stick to 2.60.3 if possible. If you already spent some time on Matrix Auth benchmarks, we can definitely suggest them in Matrix Auth plugin later

AbhyudayaSharma · 2019-06-04T05:45:04Z

My bad about the matrix auth suggestion. Let's stick to 2.60.3 if possible. If you already spent some time on Matrix Auth benchmarks, we can definitely suggest them in Matrix Auth plugin later

Since Role Strategy and Matrix auth both have some issues, what plugin should I go with here just to verify that Configuration as Code is working?

src/test/java/jenkins/jmh/benchmarks/CascStateBenchmarks.java

oleg-nenashev

The code looks good to me, just few minor comments.

I think we could proceed with documentation for the change:

Benchmarking documentation could be put directly into the repository (e.g. under docs/), no specific need to keep it elsewhere. JUnit was outside due to historical reasons
Polish Javadoc a bit so that new classes have some documentation and annotations (maybe with a link to documentation in the repo)

oleg-nenashev · 2019-06-04T06:50:33Z

.gitignore

@@ -13,3 +13,6 @@ out
 .classpath
 .project
 build
+
+# reports produced from benchmarks
+jmh-benchmark-report.json


it might be better to move the default output to target/jmh-reports/* or so so that it follows the convention used in the Maven Surefire Plugin

Done though I had to create the directory from the test method itself.

src/main/java/jenkins/jmh/BenchmarkFinder.java

src/test/java/jenkins/jmh/benchmarks/CascStateBenchmarks.java

src/test/java/jenkins/jmh/benchmarks/JmhStateBenchmark.java

src/main/java/jenkins/jmh/BenchmarkFinder.java

oleg-nenashev · 2019-06-04T07:20:20Z

Incrementals do not seem to work anymore, I will try to figure out the reason.
CC @jglick

[2019-06-04T06:27:31.476Z] Skipping deployment as no artifacts were found with the expected path, typically due to a PR merge build not up to date with its base branch: https://ci.jenkins.io/job/Core/job/jenkins-test-harness/job/PR-135/3/artifact/**/*-rc*.eaa538d48663/*-rc*.eaa538d48663*/*zip*/archive.zip

jglick · 2019-06-04T14:11:57Z

typically due to a PR merge build not up to date with its base branch

check this ^^^

AbhyudayaSharma · 2019-06-05T04:29:21Z

Unrelated failure:

java.io.IOException: Failed to run image 'maven:3.6.1-jdk-8'. Error: docker: Error response from daemon: mkdir /var/lib/docker/overlay2/c35038806833e92d002ad312b4f1335cea820816633af081832183018be2ad78-init: no space left on device.

pom.xml

oleg-nenashev

Looks good to me overall, but we need a decision w.r.t inclusion JCasC code here: https://groups.google.com/d/msg/jenkinsci-dev/cgbYEB3c_Vc/dmV6x0oQAgAJ

docs/jmh-benchmarks.adoc

oleg-nenashev · 2019-06-07T07:30:19Z

Retriggerring the build

jglick

see discussion on dev list

pom.xml

AbhyudayaSharma · 2019-06-07T14:14:54Z

Sorry, I missed it while doing a cleanup of Configuration as code from this pull request.

addressed

oleg-nenashev

Thanks @AbhyudayaSharma ! I discovered one binary compatibility breakage which should be restored imho.

Maybe it also worth moving new static methods from JenkinsRule to a new static class like JenkinsTestHelper , but this is an implementation detail. I do not require changing it

src/main/java/org/jvnet/hudson/test/JenkinsRule.java

oleg-nenashev · 2019-06-07T14:27:05Z

src/main/java/org/jvnet/hudson/test/JenkinsRule.java

@@ -434,7 +428,15 @@ public void before() throws Throwable {
     * Configures the update center setting for the test.
     * By default, we load updates from local proxy to avoid network traffic as much as possible.
     */
-    protected void configureUpdateCenter() throws Exception {


Protected methods can be overridden in other libraries. This change breaks binary compatibility from what I see

This method did exist but was not doing anything. Thanks for letting me know!

AFAICT it was not fixed, right?
Usage:

jenkins-test-harness/src/main/java/org/jvnet/hudson/test/HudsonTestCase.java

Line 337 in e72678d

configureUpdateCenter();

AbhyudayaSharma · 2019-06-07T15:37:50Z

Protected methods can be overridden in other libraries. This change breaks binary compatibility from what I see

@oleg-nenashev The method was there but it wasn't being used anywhere so overriding would have made no difference. Thanks a lot for letting me know!

Co-Authored-By: Oleg Nenashev <o.v.nenashev@gmail.com>

AbhyudayaSharma · 2019-06-10T15:32:57Z

Force pushed to rebase and enable incrementals

oleg-nenashev · 2019-06-11T07:42:24Z

@jenkinsci/jenkins-test-harness-developers I would like to merge it if nobody is against

oleg-nenashev · 2019-06-11T07:51:27Z

And CC @jenkinsci/core, because the group above is empty

oleg-nenashev · 2019-06-12T07:25:06Z

My plan is to merge it by EoD if there is no negative feedback. I checked with @jglick offline, he is fine if I do so (it is a development tool at the end of the day, and its versions can be easily pinned if something goes wrong)

jglick · 2019-06-12T13:17:50Z

My general preference is for things like this to live in their own components. Is there a particular reason this needs to be part of jenkins-test-harness? I see some existing classes being patched but it looks like that is just about opening up access to some Java members for reusability.

jglick

I do not care much about code quality of the benchmark stuff (it is experimental new code after all); focusing on code that was changed in normal JenkinsRule usage.

jglick · 2019-06-12T13:20:39Z

src/main/java/org/jvnet/hudson/test/JenkinsRule.java


+        jenkins.setCrumbIssuer(new TestCrumbIssuer());  // TODO: Move to _configureJenkinsForTest after JENKINS-55240


I asked for information in JIRA and got none. What is the issue?

When we use configuration as code to setup the Jenkins instance with TestCrumbIssuer, it was refused to be marshaled. This was happening when the code was in the Role Strategy Plugin.

use configuration as code to setup the Jenkins instance

So, TestCrumbIssuer not being loaded from the usual location I suppose? Normally it works because of this clause.

src/main/java/org/jvnet/hudson/test/JenkinsRule.java

AbhyudayaSharma · 2019-06-13T04:07:12Z

@jglick I have used addSuppressed() for the recording the ignored exceptions. However, I just wanted to let you know that they were part of JenkinsRule. Please see the current master:

jenkins-test-harness/src/main/java/org/jvnet/hudson/test/JenkinsRule.java

Lines 486 to 499 in 5e8843a

    
           jettyLevel(Level.WARNING); 
        
           try { 
        
               server.stop(); 
        
           } catch (Exception e) { 
        
               // ignore 
        
           } finally { 
        
               jettyLevel(Level.INFO); 
        
           } 
        
           for (LenientRunnable r : tearDowns) 
        
               try { 
        
                   r.run(); 
        
               } catch (Exception e) { 
        
                   // ignore 
        
               }

Thanks for the review! I hope I have answered your comments.

AbhyudayaSharma · 2019-06-13T04:39:26Z

[2019-06-13T04:29:32.713Z] [ERROR] /home/jenkins/workspace/Core_jenkins-test-harness_PR-135/src/main/java/org/jvnet/hudson/test/JenkinsRule.java:[762,20] non-static variable localPort cannot be referenced from a static context

I don't know what is happening here. Lines 762 and 763 from the latest commit don't even contain the localPort variable.

jenkins-test-harness/src/main/java/org/jvnet/hudson/test/JenkinsRule.java

Lines 760 to 765 in 998d75d

    
           if (System.getProperty("port")!=null) 
        
               connector.setPort(Integer.parseInt(System.getProperty("port"))); 
        
           server.addConnector(connector); 
        
           server.start();

AbhyudayaSharma · 2019-06-13T04:55:25Z

It's a merge conflict with #132

oleg-nenashev · 2019-06-13T07:01:23Z

@jglick

My general preference is for things like this to live in their own components. Is there a particular reason this needs to be part of jenkins-test-harness? I see some existing classes being patched but it looks like that is just about opening up access to some Java members for reusability.

It allows adding features to plugin POM and Pipeline library, see downstream PRs (will add them to the description). Basically the intention here is to make it easily accessible to plugin developers. We could have created a new "jenkins-performance-test-harness" library ofc, but it would be just another component which would be hard to discover. JMH is built on the top of JUnit, so for me it looks better to just keep the functionality as a part of JTH.

Side Note: There is a PR to JCasC: jenkinsci/configuration-as-code-plugin#921. If the request is to split this code, we will likely need to discuss splitting code there as well. JCasC test automation will be in all major plugins soon, so splitting framework here and keeping it single on the JCasC side would defeat the purpose of splitting.

jglick · 2019-06-13T13:03:23Z

let you know that they were part of JenkinsRule

Missed that, sorry. A lot of times when code gets moved around I wind up commenting on things that were actually years old!

src/main/java/org/jvnet/hudson/test/JenkinsRule.java

oleg-nenashev · 2019-06-13T13:51:33Z

Thanks @jglick !

oleg-nenashev self-requested a review June 3, 2019 12:35

oleg-nenashev reviewed Jun 3, 2019

View reviewed changes

pom.xml Outdated Show resolved Hide resolved

oleg-nenashev reviewed Jun 3, 2019

View reviewed changes

src/main/java/jenkins/jmh/JmhBenchmarkState.java Outdated Show resolved Hide resolved

oleg-nenashev reviewed Jun 3, 2019

View reviewed changes

src/main/java/jenkins/jmh/JmhBenchmarkState.java Outdated Show resolved Hide resolved

src/main/java/jenkins/jmh/JmhBenchmarkState.java Outdated Show resolved Hide resolved

AbhyudayaSharma commented Jun 4, 2019

View reviewed changes

pom.xml Outdated Show resolved Hide resolved

AbhyudayaSharma commented Jun 4, 2019

View reviewed changes

src/test/java/jenkins/jmh/benchmarks/CascStateBenchmarks.java Outdated Show resolved Hide resolved

oleg-nenashev reviewed Jun 4, 2019

View reviewed changes

oleg-nenashev added the enhancement label Jun 4, 2019

oleg-nenashev mentioned this pull request Jun 4, 2019

Enable deployment of Incremental releases #131

Merged

AbhyudayaSharma mentioned this pull request Jun 5, 2019

[JENKINS-57425] Add profile to run benchmarks only when the benchmark property is set jenkinsci/plugin-pom#206

Merged

oleg-nenashev reviewed Jun 5, 2019

View reviewed changes

pom.xml Outdated Show resolved Hide resolved

oleg-nenashev reviewed Jun 7, 2019

View reviewed changes

docs/jmh-benchmarks.adoc Outdated Show resolved Hide resolved

docs/jmh-benchmarks.adoc Show resolved Hide resolved

oleg-nenashev closed this Jun 7, 2019

oleg-nenashev reopened this Jun 7, 2019

oleg-nenashev mentioned this pull request Jun 7, 2019

Introduce a new JCasC Test Harness Library jenkinsci/configuration-as-code-plugin#917

Closed

1 task

jglick previously requested changes Jun 7, 2019

View reviewed changes

pom.xml Outdated Show resolved Hide resolved

AbhyudayaSharma force-pushed the jmh branch from ff20de4 to 61b045d Compare June 7, 2019 14:13

oleg-nenashev requested a review from jglick June 7, 2019 14:24

oleg-nenashev requested changes Jun 7, 2019

View reviewed changes

AbhyudayaSharma and others added 2 commits June 10, 2019 21:01

Add @SInCE TODO

0849d76

Co-Authored-By: Oleg Nenashev <o.v.nenashev@gmail.com>

Bring back configureUpdateCenter

d591189

AbhyudayaSharma force-pushed the jmh branch from 29e3c8d to d591189 Compare June 10, 2019 15:31

AbhyudayaSharma mentioned this pull request Jun 11, 2019

[JENKINS-57928] Allow configuring JMH benchmarks using CASC jenkinsci/configuration-as-code-plugin#921

Merged

5 tasks

oleg-nenashev requested a review from a team June 11, 2019 15:36

raul-arabaolaza approved these changes Jun 11, 2019

View reviewed changes

jglick reviewed Jun 12, 2019

View reviewed changes

Cleanup error handling in JenkinsRule.java

998d75d

AbhyudayaSharma closed this Jun 13, 2019

AbhyudayaSharma reopened this Jun 13, 2019

Merge branch 'master' into jmh

21b144d

Resolve undetected merge conflict

9827168

jglick approved these changes Jun 13, 2019

View reviewed changes

src/main/java/org/jvnet/hudson/test/JenkinsRule.java Outdated Show resolved Hide resolved

Add message to RuntimeException in JenkinsRule#_stopJenkins

60313c9

oleg-nenashev merged commit b0a2310 into jenkinsci:master Jun 13, 2019

AbhyudayaSharma deleted the jmh branch June 13, 2019 14:33

jglick mentioned this pull request Jun 17, 2019

[JENKINS-48885] - Fix getPlugin() in JenkinsRule #92

Draft

dwnusbaum mentioned this pull request Jun 24, 2019

[JENKINS-44892] CustomDescribableModel can address SCMSourceRetriever.scm.id problem jenkinsci/workflow-cps-global-lib-plugin#65

Merged

jetersen mentioned this pull request May 31, 2020

SecretResolver support file and base64 variable expansion jenkinsci/configuration-as-code-plugin#1408

Merged

6 tasks


		jenkins.setCrumbIssuer(new TestCrumbIssuer()); // TODO: Move to _configureJenkinsForTest after JENKINS-55240

[JENKINS-57653] Introduce JMH benchmarks to Jenkins Test Harness #135

[JENKINS-57653] Introduce JMH benchmarks to Jenkins Test Harness #135

Conversation

AbhyudayaSharma commented Jun 3, 2019 • edited by oleg-nenashev Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AbhyudayaSharma commented Jun 4, 2019 • edited Loading

oleg-nenashev commented Jun 4, 2019

AbhyudayaSharma commented Jun 4, 2019

oleg-nenashev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oleg-nenashev commented Jun 4, 2019

jglick commented Jun 4, 2019

AbhyudayaSharma commented Jun 5, 2019

oleg-nenashev left a comment

Choose a reason for hiding this comment

oleg-nenashev commented Jun 7, 2019

jglick left a comment

Choose a reason for hiding this comment

AbhyudayaSharma commented Jun 7, 2019

oleg-nenashev left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AbhyudayaSharma commented Jun 7, 2019

AbhyudayaSharma commented Jun 10, 2019

oleg-nenashev commented Jun 11, 2019

oleg-nenashev commented Jun 11, 2019

oleg-nenashev commented Jun 12, 2019

jglick commented Jun 12, 2019

jglick left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AbhyudayaSharma commented Jun 13, 2019

AbhyudayaSharma commented Jun 13, 2019

AbhyudayaSharma commented Jun 13, 2019 • edited Loading

oleg-nenashev commented Jun 13, 2019

jglick commented Jun 13, 2019

oleg-nenashev commented Jun 13, 2019

AbhyudayaSharma commented Jun 3, 2019 •

edited by oleg-nenashev

Loading

AbhyudayaSharma commented Jun 4, 2019 •

edited

Loading

oleg-nenashev left a comment •

edited

Loading

AbhyudayaSharma commented Jun 13, 2019 •

edited

Loading