Fix for [JENKINS-18950] issue (reports => running out of memory).

Fixed problem of running out of memory under heavy use like ours: - reports not kept in memory anymore but made on demand; - logging changes due to unserializable listener-logger.
jenkinsci · Jul 26, 2013 · 0e9529f · 0e9529f
1 parent cabe013
commit 0e9529f
Show file tree

Hide file tree

Showing 10 changed files with 74 additions and 50 deletions.
diff --git a/src/main/java/org/jenkinsci/plugins/valgrind/ValgrindBuildAction.java b/src/main/java/org/jenkinsci/plugins/valgrind/ValgrindBuildAction.java
@@ -78,7 +78,7 @@ public String getUrlName()
 	}
 
 	@Override
-	public void doGraph(StaplerRequest req, StaplerResponse rsp) throws IOException
+	public void doGraph(StaplerRequest req, StaplerResponse rsp) throws IOException, InterruptedException
 	{
 		if (ChartUtil.awtProblemCause != null)
 		{
@@ -97,7 +97,12 @@ public void doGraph(StaplerRequest req, StaplerResponse rsp) throws IOException
 		g.doPng(req, rsp);
 	}
 
-	private DataSetBuilder<String, ChartUtil.NumberOnlyBuildLabel> getDataSetBuilder()
+	/**
+	 * @return a DataSetBuilder
+	 * @throws IOException
+	 * @throws InterruptedException
+	 */
+	private DataSetBuilder<String, ChartUtil.NumberOnlyBuildLabel> getDataSetBuilder() throws IOException, InterruptedException
 	{
 		DataSetBuilder<String, ChartUtil.NumberOnlyBuildLabel> dsb = new DataSetBuilder<String, ChartUtil.NumberOnlyBuildLabel>();
 

diff --git a/src/main/java/org/jenkinsci/plugins/valgrind/ValgrindPublisher.java b/src/main/java/org/jenkinsci/plugins/valgrind/ValgrindPublisher.java
@@ -106,13 +106,12 @@ public boolean perform(AbstractBuild<?, ?> build, Launcher launcher, BuildListen
 
 		EnvVars env = build.getEnvironment();
 
-		ValgrindLogger.log(listener, "Analysing valgrind results");		
+		ValgrindLogger.log(listener, "Analysing valgrind results; configure Jenkins system log (ValgrindLogger) for details");
 
-		ValgrindParserResult parser = new ValgrindParserResult(listener, env.expand(valgrindPublisherConfig.getPattern()));
-		ValgrindReport valgrindReport;
+		ValgrindParserResult parser = new ValgrindParserResult(env.expand(valgrindPublisherConfig.getPattern()));
 
-		valgrindReport = build.getWorkspace().act(parser);		
-		ValgrindResult valgrindResult = new ValgrindResult(build, valgrindReport);		
+		ValgrindResult valgrindResult = new ValgrindResult(build, parser);
+		ValgrindReport valgrindReport = valgrindResult.getReport();
 
 		new ValgrindEvaluator(valgrindPublisherConfig, listener).evaluate(valgrindReport, build, env); 
 

diff --git a/src/main/java/org/jenkinsci/plugins/valgrind/ValgrindResult.java b/src/main/java/org/jenkinsci/plugins/valgrind/ValgrindResult.java
@@ -10,6 +10,7 @@
 import org.jenkinsci.plugins.valgrind.model.ValgrindError;
 import org.jenkinsci.plugins.valgrind.model.ValgrindProcess;
 import org.jenkinsci.plugins.valgrind.model.ValgrindReport;
+import org.jenkinsci.plugins.valgrind.parser.ValgrindParserResult;
 import org.jenkinsci.plugins.valgrind.util.ValgrindSourceFile;
 import org.jenkinsci.plugins.valgrind.util.ValgrindSummary;
 import org.kohsuke.stapler.StaplerRequest;
@@ -20,16 +21,18 @@ public class ValgrindResult implements Serializable
 {	
 	private static final long serialVersionUID = -5347879997716170059L;
 	private static final String PID_TOKEN = "pid=";
-
-	private ValgrindReport report;
+
+    private ValgrindParserResult parser;
+    private ValgrindReport report;
     private AbstractBuild<?, ?> owner;
     private Map<String, String> sourceFiles;
 
 
-    public ValgrindResult( AbstractBuild<?, ?> build, ValgrindReport report )
+    public ValgrindResult( AbstractBuild<?, ?> build, ValgrindParserResult parser)
     {
     	this.owner = build;
-    	this.report = report;
+        this.parser = parser;
+        this.report = null; //for results serialized through this plugin version and later
     }
 
 	public AbstractBuild<?, ?> getOwner()
@@ -46,16 +49,21 @@ public ValgrindPublisherConfig getConfig()
 		return action.getConfig();    			
 	}
 
-	public ValgrindReport getReport()
+	/**
+	 * @return a ValgrindReport
+	 * @throws IOException
+	 * @throws InterruptedException
+	 */
+	public ValgrindReport getReport() throws IOException, InterruptedException
 	{
-		return report;		
+	    if(report != null) {
+	        return report; //for results serialized through older plugin version(s)
+	    }
+	    else {
+	        return owner.getWorkspace().act(parser);
+	    }
 	}
 
-	public void setReport(ValgrindReport report)
-	{
-		this.report = report;
-	}
-
 	public Map<String, String> getSourceFiles()
 	{
 		return sourceFiles;
@@ -70,8 +78,10 @@ public void setSourceFiles(Map<String, String> sourceFiles)
 	 * Renders the summary Valgrind report for the build result.
 	 * 
 	 * @return the HTML fragment of the summary Valgrind report
+	 * @throws InterruptedException
+	 * @throws IOException
 	 */
-	public String getSummary()
+	public String getSummary() throws IOException, InterruptedException
 	{
 		return ValgrindSummary.createReportSummary(this);
 	}
@@ -81,11 +91,12 @@ public String getSummary()
 	 * @param link expected to be in format "id=<executable name>,<unique error id>"
 	 * @param request
 	 * @param response
-	 * @return
+	 * @return valgrind detail(s)
 	 * @throws IOException
+	 * @throws InterruptedException
 	 */
 	public Object getDynamic(final String l, final StaplerRequest request, final StaplerResponse response)
-			throws IOException
+			throws IOException, InterruptedException
 	{	
 		final String[] s = l.split("/");
 		final String data = s[s.length -1];
@@ -95,6 +106,7 @@ public Object getDynamic(final String l, final StaplerRequest request, final Sta
 
 		int sep = data.indexOf(",");
 
+		ValgrindReport report = getReport();
 		if ( sep > PID_TOKEN.length() )
 		{
 			String pid = data.substring(PID_TOKEN.length(), sep);

diff --git a/src/main/java/org/jenkinsci/plugins/valgrind/parser/ValgrindParserResult.java b/src/main/java/org/jenkinsci/plugins/valgrind/parser/ValgrindParserResult.java
@@ -2,7 +2,6 @@
 
 import hudson.FilePath;
 import hudson.Util;
-import hudson.model.BuildListener;
 import hudson.remoting.VirtualChannel;
 
 import java.io.File;
@@ -17,32 +16,30 @@
 public class ValgrindParserResult implements FilePath.FileCallable<ValgrindReport>
 {
 	private static final long serialVersionUID = -5475538646374717099L;
-	private final BuildListener listener;
 	private String pattern;
 
-	public ValgrindParserResult( final BuildListener listener, String pattern )
+	public ValgrindParserResult( String pattern )
 	{
-		this.listener = listener;
 		this.pattern = pattern;
 	}
 
 	public ValgrindReport invoke(File basedir, VirtualChannel channel) throws IOException, InterruptedException
 	{
-		ValgrindLogger.log(listener, "looking for valgrind files in '" + basedir.getAbsolutePath() + "' with pattern '" + pattern + "'");
+		ValgrindLogger.logFine("looking for valgrind files in '" + basedir.getAbsolutePath() + "' with pattern '" + pattern + "'");
 
 		ValgrindReport valgrindReport = new ValgrindReport();
 
 		for ( String fileName : findValgrindsReports( basedir ) )
 		{
-			ValgrindLogger.log(listener, "parsing " + fileName + "...");
+			ValgrindLogger.logFine("parsing " + fileName + "...");
 			try
 			{
-				ValgrindReport report = new ValgrindSaxParser(listener).parse( new File(basedir, fileName) );				
+				ValgrindReport report = new ValgrindSaxParser().parse( new File(basedir, fileName) );
 				valgrindReport.integrate( report );
 			} 
 			catch (Exception e)
 			{
-				ValgrindLogger.log(listener, "failed to parse " + fileName + ": " + e.getMessage());
+				ValgrindLogger.logFine("failed to parse " + fileName + ": " + e.getMessage());
 			}
 		}
 

diff --git a/src/main/java/org/jenkinsci/plugins/valgrind/parser/ValgrindSaxParser.java b/src/main/java/org/jenkinsci/plugins/valgrind/parser/ValgrindSaxParser.java
@@ -1,7 +1,5 @@
 package org.jenkinsci.plugins.valgrind.parser;
 
-import hudson.model.BuildListener;
-
 import java.io.File;
 import java.io.IOException;
 import java.io.Serializable;
@@ -26,7 +24,6 @@
 public class ValgrindSaxParser implements Serializable
 {
 	private static final long serialVersionUID = -6889121223670989851L;
-	private BuildListener listener;
 
 	private class Handler extends DefaultHandler
 	{
@@ -143,7 +140,7 @@ public void endElement(String uri, String localName, String qName) throws SAXExc
 				}
 				catch( IllegalArgumentException e )
 				{
-					ValgrindLogger.log(listener, "WARNING: Valgrind error not supported: " + data.toString());					
+					ValgrindLogger.logWarn("Valgrind error not supported: " + data.toString());
 				}				
 			}
 
@@ -176,7 +173,7 @@ public void endElement(String uri, String localName, String qName) throws SAXExc
 				}
 				catch( NumberFormatException e )
 				{
-					ValgrindLogger.log(listener, "WARNING: '" + data.toString() + "' is not a valid number of leaked bytes");
+					ValgrindLogger.logWarn("'" + data.toString() + "' is not a valid number of leaked bytes");
 				}
 			}
 
@@ -188,7 +185,7 @@ public void endElement(String uri, String localName, String qName) throws SAXExc
 				}
 				catch( NumberFormatException e )
 				{
-					ValgrindLogger.log(listener, "WARNING: '" + data.toString() + "' is not a valid number of leaked blocks");
+					ValgrindLogger.logWarn("'" + data.toString() + "' is not a valid number of leaked blocks");
 				}
 			}
 
@@ -257,11 +254,6 @@ public ValgrindReport getReport()
 		}
 	}
 
-	public ValgrindSaxParser(BuildListener listener)
-	{
-		this.listener = listener;
-	}
-
 	public ValgrindReport parse( final File file ) throws ParserConfigurationException, SAXException, IOException
 	{
 		SAXParserFactory factory = SAXParserFactory.newInstance();

diff --git a/src/main/java/org/jenkinsci/plugins/valgrind/util/AbstractValgrindBuildAction.java b/src/main/java/org/jenkinsci/plugins/valgrind/util/AbstractValgrindBuildAction.java
@@ -40,6 +40,12 @@ public <T extends AbstractValgrindBuildAction> T getPreviousResult() {
         return owner;
     }
 
-    public abstract void doGraph(StaplerRequest req, StaplerResponse rsp) throws IOException;
+    /**
+     * @param request
+     * @param response
+     * @throws IOException
+     * @throws InterruptedException
+     */
+    public abstract void doGraph(StaplerRequest req, StaplerResponse rsp) throws IOException, InterruptedException;
 
 }
diff --git a/src/main/java/org/jenkinsci/plugins/valgrind/util/AbstractValgrindProjectAction.java b/src/main/java/org/jenkinsci/plugins/valgrind/util/AbstractValgrindProjectAction.java
@@ -6,7 +6,6 @@
 import org.kohsuke.stapler.StaplerRequest;
 import org.kohsuke.stapler.StaplerResponse;
 
-
 import hudson.model.AbstractBuild;
 import hudson.model.AbstractProject;
 import hudson.model.Actionable;
@@ -40,7 +39,7 @@ public String getSearchUrl()
 
     protected abstract Integer getLastResultBuild();
 
-	public void doGraph(StaplerRequest req, StaplerResponse rsp) throws IOException
+	public void doGraph(StaplerRequest req, StaplerResponse rsp) throws IOException, InterruptedException
 	{
 		AbstractBuild<?, ?> lastBuild = getLastFinishedBuild();
 		ValgrindBuildAction valgrindBuildAction = lastBuild.getAction(ValgrindBuildAction.class);

diff --git a/src/main/java/org/jenkinsci/plugins/valgrind/util/ValgrindLogger.java b/src/main/java/org/jenkinsci/plugins/valgrind/util/ValgrindLogger.java
@@ -2,10 +2,24 @@
 
 import hudson.model.BuildListener;
 
+import java.util.logging.Logger;
+
 public abstract class ValgrindLogger
 {
-	public static void log(BuildListener listener, final String message)
+    private static final Logger LOGGER = Logger.getLogger(ValgrindLogger.class.getSimpleName());
+
+    public static void log(BuildListener listener, final String message)
 	{
 		listener.getLogger().println("[Valgrind] " + message);
 	}
+
+    public static void logFine(final String message)
+    {
+        LOGGER.fine(message);
+    }
+
+    public static void logWarn(final String message)
+    {
+        LOGGER.warning(message);
+    }
 }
diff --git a/src/main/java/org/jenkinsci/plugins/valgrind/util/ValgrindSummary.java b/src/main/java/org/jenkinsci/plugins/valgrind/util/ValgrindSummary.java
@@ -1,5 +1,7 @@
 package org.jenkinsci.plugins.valgrind.util;
 
+import java.io.IOException;
+
 import org.jenkinsci.plugins.valgrind.ValgrindResult;
 
 public class ValgrindSummary
@@ -10,8 +12,10 @@ public class ValgrindSummary
 	 * @param result
 	 *            the valgrind result object
 	 * @return the HTML fragment representing the valgrind report summary
+	 * @throws InterruptedException
+	 * @throws IOException
 	 */
-	public static String createReportSummary(ValgrindResult result)
+	public static String createReportSummary(ValgrindResult result) throws IOException, InterruptedException
 	{
 
 		StringBuilder summary = new StringBuilder();

diff --git a/src/test/java/org/jenkinsci/plugins/valgrind/parser/ValgrindSaxParserTest.java b/src/test/java/org/jenkinsci/plugins/valgrind/parser/ValgrindSaxParserTest.java
@@ -3,15 +3,13 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
-import hudson.model.BuildListener;
 
 import java.io.File;
 import java.io.IOException;
 import java.util.List;
 
 import javax.xml.parsers.ParserConfigurationException;
 
-import org.easymock.EasyMock;
 import org.jenkinsci.plugins.valgrind.model.ValgrindAuxiliary;
 import org.jenkinsci.plugins.valgrind.model.ValgrindError;
 import org.jenkinsci.plugins.valgrind.model.ValgrindErrorKind;
@@ -25,14 +23,12 @@
 
 public class ValgrindSaxParserTest
 {
-	private BuildListener listenerMock;
 	private ValgrindSaxParser parser;
 
 	@Before
 	public void setup()
 	{
-		listenerMock = EasyMock.createMock(BuildListener.class);
-		parser = new ValgrindSaxParser(listenerMock);
+		parser = new ValgrindSaxParser();
 	}
 
 	@Test