Merge pull request #1843 from olivergondza/JENKINS-30705

[FIXED JENKINS-30705] Optimize TagCloud calculation
jenkinsci · Oct 1, 2015 · fca99dc · fca99dc
2 parents 8c12240 + 52f9172
commit fca99dc
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 12 deletions.
diff --git a/core/src/main/java/hudson/model/Label.java b/core/src/main/java/hudson/model/Label.java
@@ -85,6 +85,7 @@ public abstract class Label extends Actionable implements Comparable<Label>, Mod
     protected transient final String name;
     private transient volatile Set<Node> nodes;
     private transient volatile Set<Cloud> clouds;
+    private transient volatile int tiedJobsCount;
 
     @Exported
     public transient final LoadStatistics loadStatistics;
@@ -369,13 +370,17 @@ public List<AbstractProject> getTiedJobs() {
     }
 
     /**
-     * Returns a count of projects that are tied on this node. In a system without security this should be the same
+     * Returns an approximate count of projects that are tied on this node.
+     *
+     * In a system without security this should be the same
      * as {@code getTiedJobs().size()} but significantly faster as it involves fewer temporary objects and avoids
      * sorting the intermediary list. In a system with security, this will likely return a higher value as it counts
      * all jobs (mostly) irrespective of access.
      * @return a count of projects that are tied on this node.
      */
     public int getTiedJobCount() {
+        if (tiedJobsCount != -1) return tiedJobsCount;
+
         // denormalize for performance
         // we don't need to respect security as much when returning a simple count
         SecurityContext context = ACL.impersonate(ACL.SYSTEM);
@@ -412,7 +417,7 @@ public int getTiedJobCount() {
                     }
                 }
             }
-            return result;
+            return tiedJobsCount = result;
         } finally {
             SecurityContextHolder.setContext(context);
         }
@@ -433,6 +438,7 @@ public boolean isEmpty() {
     /*package*/ void reset() {
         nodes = null;
         clouds = null;
+        tiedJobsCount = -1;
     }
 
     /**

diff --git a/test/src/test/java/hudson/model/NodeTest.java b/test/src/test/java/hudson/model/NodeTest.java
@@ -123,11 +123,13 @@ public void testGetLabelCloud() throws Exception {
         Node node = j.createOnlineSlave();
         node.setLabelString("label1 label2");
         FreeStyleProject project = j.createFreeStyleProject();
-        project.setAssignedLabel(j.jenkins.getLabel("label1"));
+        final Label label = j.jenkins.getLabel("label1");
+        project.setAssignedLabel(label);
+        label.reset(); // Make sure cached value is not used
         TagCloud<LabelAtom> cloud = node.getLabelCloud();
         for(int i =0; i< cloud.size(); i ++){
             TagCloud.Entry e = cloud.get(i);
-            if(e.item.equals(j.jenkins.getLabel("label1"))){
+            if(e.item.equals(label)){
                 assertEquals("Label label1 should have one tied project.", 1, e.weight, 0);
             }
             else{
@@ -256,7 +258,9 @@ public void testGetAssignedLabelWithJobs() throws Exception {
         Integer labelCount = RunLoadCounter.assertMaxLoads(mavenProject, 0, new Callable<Integer>() {
             @Override
             public Integer call() throws Exception {
-                return j.jenkins.getLabel("label1").getTiedJobCount();
+                final Label label = j.jenkins.getLabel("label1");
+                label.reset(); // Make sure cached value is not used
+                return label.getTiedJobCount();
             }
         });
 
@@ -291,15 +295,16 @@ public void testGetAssignedLabelMultipleSlaves() throws Exception {
         node1.setLabelString("label1");
 
         MavenModuleSet project = j.createMavenProject();
-        project.setAssignedLabel(j.jenkins.getLabel("label1"));
+        final Label label = j.jenkins.getLabel("label1");
+        project.setAssignedLabel(label);
         j.assertBuildStatus(Result.FAILURE, project.scheduleBuild2(0).get());
 
         MavenModuleSet project2 = j.createMavenProject();
-        project2.setAssignedLabel(j.jenkins.getLabel("label1"));
+        project2.setAssignedLabel(label);
         j.assertBuildStatus(Result.FAILURE, project2.scheduleBuild2(0).get());
 
-        assertEquals("Two jobs should be tied to this label.",
-                2, j.jenkins.getLabel("label1").getTiedJobCount());
+        label.reset(); // Make sure cached value is not used
+        assertEquals("Two jobs should be tied to this label.", 2, label.getTiedJobCount());
     }
 
     /**
@@ -312,12 +317,13 @@ public void testGetAssignedLabelWhenLabelRemoveFromProject() throws Exception {
         node.setLabelString("label1");
 
         MavenModuleSet project = j.createMavenProject();
-        project.setAssignedLabel(j.jenkins.getLabel("label1"));
+        final Label label = j.jenkins.getLabel("label1");
+        project.setAssignedLabel(label);
         j.assertBuildStatus(Result.FAILURE, project.scheduleBuild2(0).get());
 
         project.setAssignedLabel(null);
-        assertEquals("Label1 should have no tied jobs after the job label was removed.",
-                0, j.jenkins.getLabel("label1").getTiedJobCount());
+        label.reset(); // Make sure cached value is not used
+        assertEquals("Label1 should have no tied jobs after the job label was removed.", 0, label.getTiedJobCount());
     }
 
     /**