[FIXED JENKINS-22822] PeepholePermalink should maintain an in-memory …

…cache of symlinks to avoid disk I/O.
jenkinsci · May 28, 2014 · 2ff7a90 · 2ff7a90
1 parent 8a79907
commit 2ff7a90
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 5 deletions.
diff --git a/changelog.html b/changelog.html
@@ -58,6 +58,9 @@
   <li class="bug">
     Fixed a reference counting bug in the remoting layer.
     (<a href="https://github.com/jenkinsci/remoting/commit/387433c98b66bf453e27198973daa758787095b3">commit</a>)
+  <li class="bug">
+    Avoid repeatedly reading symlinks from disk to resolve build permalinks.
+    (<a href="https://issues.jenkins-ci.org/browse/JENKINS-22822">issue 22822</a>)
   <li class="bug">
     Show custom build display name in executors widget.
     (<a href="https://issues.jenkins-ci.org/browse/JENKINS-10477">issue 10477</a>)

diff --git a/core/src/main/java/jenkins/model/PeepholePermalink.java b/core/src/main/java/jenkins/model/PeepholePermalink.java
@@ -10,16 +10,17 @@
 import hudson.model.listeners.RunListener;
 import hudson.util.AtomicFileWriter;
 import hudson.util.StreamTaskListener;
-import org.apache.commons.io.FileUtils;
-
-import javax.annotation.Nonnull;
-import javax.annotation.Nullable;
 import java.io.File;
 import java.io.IOException;
 import java.io.StringWriter;
 import java.util.Arrays;
+import java.util.HashMap;
+import java.util.Map;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+import org.apache.commons.io.FileUtils;
 
 /**
  * Convenient base implementation for {@link Permalink}s that satisfy
@@ -59,6 +60,10 @@
  * @since 1.507
  */
 public abstract class PeepholePermalink extends Permalink implements Predicate<Run<?,?>> {
+
+    /** JENKINS-22822: avoids rereading symlinks */
+    static final Map<File,String> symlinks = new HashMap<File,String>();
+
     /**
      * Checks if the given build satisfies the peep-hole criteria.
      *
@@ -139,7 +144,7 @@ protected void updateCache(@Nonnull Job<?,?> job, @Nullable Run<?,?> b) {
                 // (re)create the build Number->Id symlink
                 Util.createSymlink(job.getBuildDir(),b.getId(),target,TaskListener.NULL);
             }
-            writeSymlink(cache, String.valueOf(n));
+            writeSymlink(cache, target);
         } catch (IOException e) {
             LOGGER.log(Level.WARNING, "Failed to update "+job+" "+getId()+" permalink for " + b, e);
             cache.delete();
@@ -156,15 +161,30 @@ private static boolean exists(File link) {
     }
 
     static String readSymlink(File cache) throws IOException, InterruptedException {
+        synchronized (symlinks) {
+            String target = symlinks.get(cache);
+            if (target != null) {
+                LOGGER.log(Level.FINE, "readSymlink cached {0} → {1}", new Object[] {cache, target});
+                return target;
+            }
+        }
         String target = Util.resolveSymlink(cache);
         if (target==null && cache.exists()) {
             // if this file isn't a symlink, it must be a regular file
             target = FileUtils.readFileToString(cache,"UTF-8").trim();
         }
+        LOGGER.log(Level.FINE, "readSymlink {0} → {1}", new Object[] {cache, target});
+        synchronized (symlinks) {
+            symlinks.put(cache, target);
+        }
         return target;
     }
 
     static void writeSymlink(File cache, String target) throws IOException, InterruptedException {
+        LOGGER.log(Level.FINE, "writeSymlink {0} → {1}", new Object[] {cache, target});
+        synchronized (symlinks) {
+            symlinks.put(cache, target);
+        }
         StringWriter w = new StringWriter();
         StreamTaskListener listener = new StreamTaskListener(w);
         Util.createSymlink(cache.getParentFile(),target,cache.getName(),listener);

diff --git a/core/src/test/java/jenkins/model/PeepholePermalinkTest.java b/core/src/test/java/jenkins/model/PeepholePermalinkTest.java
@@ -39,6 +39,7 @@ public class PeepholePermalinkTest {
     @Test public void symlinks() throws Exception {
         File link = new File(tmp.getRoot(), "link");
         PeepholePermalink.writeSymlink(link, "stuff");
+        PeepholePermalink.symlinks.clear(); // so we actually test the filesystem
         assertEquals("stuff", PeepholePermalink.readSymlink(link));
     }