[FIXED JENKINS-21078] Optimizing loadIdOnDisk.

jenkinsci · Dec 18, 2013 · a6e01f7 · a6e01f7
1 parent 358fe7f
commit a6e01f7
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 32 deletions.
diff --git a/changelog.html b/changelog.html
@@ -61,6 +61,9 @@
   <li class=bug>
     Better robustness against XML deserialization errors.
     (<a href="https://issues.jenkins-ci.org/browse/JENKINS-21024">issue 21024</a>)
+  <li class=bug>
+    Minimizing disk I/O while loading the <em>names</em> of build records during Jenkins startup.
+    (<a href="https://issues.jenkins-ci.org/browse/JENKINS-21078">issue 21078</a>)
   <li class=bug>
     Avoiding serializing the owning build as part of a test result action, as this can lead to errors later.
     (<a href="https://issues.jenkins-ci.org/browse/JENKINS-18410">issue 18410</a>)

diff --git a/core/src/main/java/hudson/model/RunMap.java b/core/src/main/java/hudson/model/RunMap.java
@@ -194,24 +194,21 @@ protected FilenameFilter createDirectoryFilter() {
         final SimpleDateFormat formatter = Run.ID_FORMATTER.get();
 
         return new FilenameFilter() {
-            public boolean accept(File dir, String name) {
-                // JENKINS-1461 sometimes create bogus data directories with impossible dates, such as year 0, April 31st,
-                // or August 0th. Date object doesn't roundtrip those, so we eventually fail to load this data.
-                // Don't even bother trying.
-                if (!isCorrectDate(name)) {
-                    LOGGER.log(FINE, "Skipping {0}", new File(dir,name));
+            @Override public boolean accept(File dir, String name) {
+                if (name.startsWith("0000")) {
+                    // JENKINS-1461 sometimes create bogus data directories with impossible dates, such as year 0, April 31st,
+                    // or August 0th. Date object doesn't roundtrip those, so we eventually fail to load this data.
+                    // Don't even bother trying.
                     return false;
                 }
-                return !name.startsWith("0000") && new File(dir,name).isDirectory();
-            }
-
-            private boolean isCorrectDate(String name) {
                 try {
-                    if(formatter.format(formatter.parse(name)).equals(name))
+                    if (formatter.format(formatter.parse(name)).equals(name)) {
                         return true;
+                    }
                 } catch (ParseException e) {
                     // fall through
                 }
+                LOGGER.log(FINE, "Skipping {0} in {1}", new Object[] {name, dir});
                 return false;
             }
         };

diff --git a/core/src/main/java/jenkins/model/lazy/AbstractLazyLoadRunMap.java b/core/src/main/java/jenkins/model/lazy/AbstractLazyLoadRunMap.java
@@ -26,19 +26,15 @@
 import hudson.model.Job;
 import hudson.model.Run;
 import hudson.model.RunMap;
-import org.apache.commons.collections.keyvalue.DefaultMapEntry;
-import org.kohsuke.accmod.Restricted;
-import org.kohsuke.accmod.restrictions.NoExternalUse;
-
 import java.io.File;
 import java.io.FilenameFilter;
 import java.io.IOException;
 import java.lang.ref.Reference;
 import java.util.AbstractMap;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collections;
 import java.util.Comparator;
+import java.util.List;
 import java.util.Map;
 import java.util.NoSuchElementException;
 import java.util.Set;
@@ -50,6 +46,9 @@
 
 import static jenkins.model.lazy.AbstractLazyLoadRunMap.Direction.*;
 import static jenkins.model.lazy.Boundary.*;
+import org.apache.commons.collections.keyvalue.DefaultMapEntry;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
 
 /**
  * {@link SortedMap} that keeps build records by their build numbers, in the descending order
@@ -224,26 +223,27 @@ public void purgeCache() {
     }
 
     private void loadIdOnDisk() {
-        String[] buildDirs = dir.list(createDirectoryFilter());
-        if (buildDirs==null) {
+        String[] kids = dir.list();
+        if (kids == null) {
             // the job may have just been created
-            buildDirs=EMPTY_STRING_ARRAY;
+            kids = EMPTY_STRING_ARRAY;
         }
-        // wrap into ArrayList to enable mutation
-        Arrays.sort(buildDirs);
-        idOnDisk = new SortedList<String>(new ArrayList<String>(Arrays.asList(buildDirs)));
-
-        // TODO: should we check that shortcuts is a symlink?
-        String[] shortcuts = dir.list();
-        if (shortcuts==null)    shortcuts=EMPTY_STRING_ARRAY;
-        SortedIntList list = new SortedIntList(shortcuts.length/2);
-        for (String s : shortcuts) {
-            try {
-                list.add(Integer.parseInt(s));
-            } catch (NumberFormatException e) {
-                // this isn't a shortcut
+        List<String> buildDirs = new ArrayList<String>();
+        FilenameFilter buildDirFilter = createDirectoryFilter();
+        SortedIntList list = new SortedIntList(kids.length / 2);
+        for (String s : kids) {
+            if (buildDirFilter.accept(dir, s)) {
+                buildDirs.add(s);
+            } else {
+                try {
+                    list.add(Integer.parseInt(s));
+                } catch (NumberFormatException e) {
+                    // this isn't a shortcut
+                }
             }
         }
+        Collections.sort(buildDirs);
+        idOnDisk = new SortedList<String>(buildDirs);
         list.sort();
         numberOnDisk = list;
     }