[FIXED JENKINS-18895] MavenModuleSetBuild.getResult is expensive.

jenkinsci · Jul 29, 2013 · d1d5248 · d1d5248
1 parent 1f23168
commit d1d5248
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 5 deletions.
diff --git a/changelog.html b/changelog.html
@@ -61,6 +61,9 @@
   <li class=bug>
     Maven build failure wasn't describing errors like Maven CLI does.
     (<a href="https://issues.jenkins-ci.org/browse/JENKINS-15025">issue 15025</a>)
+  <li class=bug>
+    <code>MavenModuleSetBuild.getResult</code> is expensive.
+    (<a href="https://issues.jenkins-ci.org/browse/JENKINS-18895">issue 18895</a>)
   <li class=bug>
     Revisited fix to be compatible for plugins.
     (<a href="https://issues.jenkins-ci.org/browse/JENKINS-18119">issue 18119</a>)

diff --git a/maven-plugin/src/main/java/hudson/maven/MavenModuleSetBuild.java b/maven-plugin/src/main/java/hudson/maven/MavenModuleSetBuild.java
@@ -53,7 +53,6 @@
 import hudson.model.Run;
 import hudson.model.StringParameterDefinition;
 import hudson.model.TaskListener;
-import hudson.remoting.Callable;
 import hudson.remoting.VirtualChannel;
 import hudson.scm.ChangeLogSet;
 import hudson.tasks.BuildStep;
@@ -93,7 +92,6 @@
 import org.apache.commons.io.FilenameUtils;
 import org.apache.commons.lang.StringUtils;
 import org.apache.maven.artifact.versioning.ComparableVersion;
-import org.apache.maven.model.building.ModelBuildingRequest;
 import org.apache.maven.project.MavenProject;
 import org.apache.maven.project.ProjectBuildingException;
 import org.codehaus.plexus.util.PathTool;
@@ -136,6 +134,7 @@ public class MavenModuleSetBuild extends AbstractMavenBuild<MavenModuleSet,Maven
     private String mavenVersionUsed;
 
     private transient Object notifyModuleBuildLock = new Object();
+    private transient Result effectiveResult;
 
     public MavenModuleSetBuild(MavenModuleSet job) throws IOException {
         super(job);
@@ -194,6 +193,11 @@ public EnvVars getEnvironment(TaskListener log) throws IOException, InterruptedE
      */
     @Override
     public Result getResult() {
+        synchronized (notifyModuleBuildLock) {
+            if (effectiveResult != null) {
+                return effectiveResult;
+            }
+        }
         Result r = super.getResult();
 
         for (MavenBuild b : getModuleLastBuilds().values()) {
@@ -208,6 +212,11 @@ public Result getResult() {
                 r = r.combine(br);
         }
 
+        synchronized (notifyModuleBuildLock) {
+            if (effectiveResult == null) {
+                effectiveResult = r;
+            }
+        }
         return r;
     }
 
@@ -522,6 +531,7 @@ public Fingerprint.RangeSet getDownstreamRelationship(@SuppressWarnings("rawtype
             // use a separate lock object since this synchronized block calls into plugins,
             // which in turn can access other MavenModuleSetBuild instances, which will result in a dead lock.
             synchronized(notifyModuleBuildLock) {
+                effectiveResult = null;
                 boolean modified = false;
 
                 List<Action> actions = getActions();

diff --git a/test/src/main/java/org/jvnet/hudson/test/RunLoadCounter.java b/test/src/main/java/org/jvnet/hudson/test/RunLoadCounter.java
@@ -50,12 +50,16 @@ public final class RunLoadCounter {
 
     /**
      * Prepares a new project to be measured.
-     * Call this <em>before</em> starting builds.
+     * Usually called before starting builds, but may also be called retroactively.
      * @param project a project of any kind
      * @throws IOException if preparations fail
      */
     public static void prepare(AbstractProject<?,?> project) throws IOException {
         project.getPublishersList().add(new MarkerAdder());
+        for (AbstractBuild<?,?> build : project._getRuns()) {
+            Marker.add(build);
+            build.save();
+        }
     }
 
     /**
@@ -97,6 +101,10 @@ private RunLoadCounter() {}
     @Restricted(NoExternalUse.class)
     public static final class Marker implements RunAction {
 
+        static void add(AbstractBuild<?,?> build) {
+            build.addAction(new Marker(build.getParent().getFullName(), build.getNumber()));
+        }
+
         private final String project;
         private final int build;
 
@@ -137,7 +145,7 @@ public static final class Marker implements RunAction {
     public static final class MarkerAdder extends Notifier {
 
         @Override public boolean perform(AbstractBuild<?, ?> build, Launcher launcher, BuildListener listener) throws InterruptedException, IOException {
-            build.addAction(new Marker(build.getParent().getFullName(), build.getNumber()));
+            Marker.add(build);
             return true;
         }
 

diff --git a/test/src/test/java/hudson/maven/MavenBuildSurefireFailedTest.java b/test/src/test/java/hudson/maven/MavenBuildSurefireFailedTest.java
@@ -2,10 +2,12 @@
 
 import hudson.model.Result;
 import hudson.tasks.Shell;
+import java.util.concurrent.Callable;
 
 import org.jvnet.hudson.test.Bug;
 import org.jvnet.hudson.test.ExtractResourceSCM;
 import org.jvnet.hudson.test.HudsonTestCase;
+import org.jvnet.hudson.test.RunLoadCounter;
 
 /**
  * @author Olivier Lamy
@@ -24,10 +26,19 @@ public void testMaven2Unstable() throws Exception {
     @Bug(8415)
     public void testMaven2Failed() throws Exception {
         configureDefaultMaven();
-        MavenModuleSet m = createMavenProject();
+        final MavenModuleSet m = createMavenProject();
         m.setGoals( "test -Dmaven.test.failure.ignore=false" );
         m.setScm(new ExtractResourceSCM(getClass().getResource("maven-multimodule-unit-failure.zip")));
         assertBuildStatus(Result.FAILURE, m.scheduleBuild2(0).get());
+        // JENKINS-18895:
+        MavenModule failing = m.getModule("com.mycompany.app:my-app");
+        assertEquals(Result.FAILURE, failing.getLastBuild().getResult());
+        RunLoadCounter.prepare(failing);
+        assertEquals(Result.FAILURE, RunLoadCounter.assertMaxLoads(failing, 0, new Callable<Result>() {
+            @Override public Result call() throws Exception {
+                return m.getLastBuild().getResult();
+            }
+        }));
     }   
 
     @Bug(8415)