[FIXED JENKINS-39535] - Optimize get log method (#2607)

* Add some tests to current behaviour of getLog method * getLog(maxLines) reads only last maxLines lines now It should speed up and reduce memory consumption for some plugins (i.e. Email-ext Plugin). Also now this method could be used to get last lines of build output in efficient manner. * Fix issues from code review
jenkinsci · Nov 6, 2016 · 2e8c3be · 2e8c3be
1 parent a57b52e
commit 2e8c3be
Show file tree

Hide file tree

Showing 2 changed files with 96 additions and 23 deletions.
diff --git a/core/src/main/java/hudson/model/Run.java b/core/src/main/java/hudson/model/Run.java
@@ -36,13 +36,16 @@
 import hudson.ExtensionPoint;
 import hudson.FeedAdapter;
 import hudson.Functions;
+import hudson.console.AnnotatedLargeText;
+import hudson.console.ConsoleLogFilter;
+import hudson.console.ConsoleNote;
+import hudson.console.ModelHyperlinkNote;
+import hudson.console.PlainTextConsoleOutputStream;
 import jenkins.util.SystemProperties;
 import hudson.Util;
 import hudson.XmlFile;
 import hudson.cli.declarative.CLIMethod;
-import hudson.console.*;
 import hudson.model.Descriptor.FormException;
-import hudson.model.Run.RunExecution;
 import hudson.model.listeners.RunListener;
 import hudson.model.listeners.SaveableListener;
 import hudson.model.queue.Executables;
@@ -58,7 +61,7 @@
 import hudson.util.LogTaskListener;
 import hudson.util.ProcessTree;
 import hudson.util.XStream2;
-import java.io.BufferedReader;
+
 import java.io.ByteArrayInputStream;
 import java.io.File;
 import java.io.FileInputStream;
@@ -68,6 +71,7 @@
 import java.io.InputStreamReader;
 import java.io.OutputStream;
 import java.io.PrintWriter;
+import java.io.RandomAccessFile;
 import java.io.Reader;
 import java.io.StringWriter;
 import java.nio.charset.Charset;
@@ -83,7 +87,6 @@
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedHashMap;
-import java.util.LinkedList;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
@@ -113,9 +116,14 @@
 import org.acegisecurity.Authentication;
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.jelly.XMLOutput;
+import org.apache.commons.lang.ArrayUtils;
 import org.kohsuke.accmod.Restricted;
 import org.kohsuke.accmod.restrictions.NoExternalUse;
-import org.kohsuke.stapler.*;
+import org.kohsuke.stapler.HttpResponse;
+import org.kohsuke.stapler.QueryParameter;
+import org.kohsuke.stapler.Stapler;
+import org.kohsuke.stapler.StaplerRequest;
+import org.kohsuke.stapler.StaplerResponse;
 import org.kohsuke.stapler.export.Exported;
 import org.kohsuke.stapler.export.ExportedBean;
 import org.kohsuke.stapler.interceptor.RequirePOST;
@@ -1930,31 +1938,54 @@ public synchronized void save() throws IOException {
      * @throws IOException If there is a problem reading the log file.
      */
     public @Nonnull List<String> getLog(int maxLines) throws IOException {
-        int lineCount = 0;
-        List<String> logLines = new LinkedList<String>();
         if (maxLines == 0) {
-            return logLines;
-        }
-        try (BufferedReader reader = new BufferedReader(new InputStreamReader(new FileInputStream(getLogFile()), getCharset()))) {
-            for (String line = reader.readLine(); line != null; line = reader.readLine()) {
-                logLines.add(line);
-                ++lineCount;
-                // If we have too many lines, remove the oldest line.  This way we
-                // never have to hold the full contents of a huge log file in memory.
-                // Adding to and removing from the ends of a linked list are cheap
-                // operations.
-                if (lineCount > maxLines)
-                    logLines.remove(0);
+            return Collections.emptyList();
+        }
+
+        int lines = 0;
+        long filePointer;
+        final List<String> lastLines = new ArrayList<>(Math.min(maxLines, 128));
+        final List<Byte> bytes = new ArrayList<>();
+
+        try (RandomAccessFile fileHandler = new RandomAccessFile(getLogFile(), "r")) {
+            long fileLength = fileHandler.length() - 1;
+
+            for (filePointer = fileLength; filePointer != -1 && maxLines != lines; filePointer--) {
+                fileHandler.seek(filePointer);
+                byte readByte = fileHandler.readByte();
+
+                if (readByte == 0x0A) {
+                    if (filePointer < fileLength) {
+                        lines = lines + 1;
+                        lastLines.add(convertBytesToString(bytes));
+                        bytes.clear();
+                    }
+                } else if (readByte != 0xD) {
+                    bytes.add(readByte);
+                }
             }
         }
 
+        if (lines != maxLines) {
+            lastLines.add(convertBytesToString(bytes));
+        }
+
+        Collections.reverse(lastLines);
+
         // If the log has been truncated, include that information.
         // Use set (replaces the first element) rather than add so that
         // the list doesn't grow beyond the specified maximum number of lines.
-        if (lineCount > maxLines)
-            logLines.set(0, "[...truncated " + (lineCount - (maxLines - 1)) + " lines...]");
+        if (lines == maxLines) {
+            lastLines.set(0, "[...truncated " + Functions.humanReadableByteSize(filePointer)+ "...]");
+        }
+
+        return ConsoleNote.removeNotes(lastLines);
+    }
 
-        return ConsoleNote.removeNotes(logLines);
+    private String convertBytesToString(List<Byte> bytes) {
+        Collections.reverse(bytes);
+        Byte[] byteArray = bytes.toArray(new Byte[bytes.size()]);
+        return new String(ArrayUtils.toPrimitive(byteArray), getCharset());
     }
 
     public void doBuildStatus( StaplerRequest req, StaplerResponse rsp ) throws IOException {

diff --git a/core/src/test/java/hudson/model/RunTest.java b/core/src/test/java/hudson/model/RunTest.java
@@ -159,7 +159,7 @@ public void getLogReturnsAnEmptyListWhenCalledWith0() throws Exception {
         Job j = Mockito.mock(Job.class);
         File tempBuildDir = tmp.newFolder();
         Mockito.when(j.getBuildDir()).thenReturn(tempBuildDir);
-        Run r = new Run(j, 0) {};
+        Run<? extends Job<?, ?>, ? extends Run<?, ?>> r = new Run(j, 0) {};
         File f = r.getLogFile();
         f.getParentFile().mkdirs();
         PrintWriter w = new PrintWriter(f, "utf-8");
@@ -169,4 +169,46 @@ public void getLogReturnsAnEmptyListWhenCalledWith0() throws Exception {
         assertTrue(logLines.isEmpty());
     }
 
+    @Test
+    public void getLogReturnsAnRightOrder() throws Exception {
+        Job j = Mockito.mock(Job.class);
+        File tempBuildDir = tmp.newFolder();
+        Mockito.when(j.getBuildDir()).thenReturn(tempBuildDir);
+        Run<? extends Job<?, ?>, ? extends Run<?, ?>> r = new Run(j, 0) {};
+        File f = r.getLogFile();
+        f.getParentFile().mkdirs();
+        PrintWriter w = new PrintWriter(f, "utf-8");
+        for (int i = 0; i < 20; i++) {
+            w.println("dummy" + i);
+        }
+
+        w.close();
+        List<String> logLines = r.getLog(10);
+        assertFalse(logLines.isEmpty());
+
+        for (int i = 1; i < 10; i++) {
+            assertEquals("dummy" + (10+i), logLines.get(i));
+        }
+        assertEquals("[...truncated 68 B...]", logLines.get(0));
+    }
+
+    @Test
+    public void getLogReturnsAllLines() throws Exception {
+        Job j = Mockito.mock(Job.class);
+        File tempBuildDir = tmp.newFolder();
+        Mockito.when(j.getBuildDir()).thenReturn(tempBuildDir);
+        Run<? extends Job<?, ?>, ? extends Run<?, ?>> r = new Run(j, 0) {};
+        File f = r.getLogFile();
+        f.getParentFile().mkdirs();
+        PrintWriter w = new PrintWriter(f, "utf-8");
+        w.print("a1\nb2\n\nc3");
+        w.close();
+        List<String> logLines = r.getLog(10);
+        assertFalse(logLines.isEmpty());
+
+        assertEquals("a1", logLines.get(0));
+        assertEquals("b2", logLines.get(1));
+        assertEquals("", logLines.get(2));
+        assertEquals("c3", logLines.get(3));
+    }
 }