Skip to content
Permalink
Browse files
[JENKINS-17363] Ludicrously slow load time [with lazyloading]
[JENKINS-19437] Implement load on demand functionality in Cppcheck

- Lazy loaded details are stored in CppcheckResult member variable and never released by Java garbace collector since a reference to them exists forever. All build.xml files are loaded during Jenkins startup and never released, CppcheckResult objects are part of them. This is kind of a memory leak. The update will cause slight lower performance but releasing of the memory is much more important for long run tasks/daemons/servers.
- Method lazyLoadSourceContainer() updated to return the data instead of their caching in a member variable.
- If cached CppcheckSourceContainer object can't be loaded, an empty one will be created. Different constructor used to remove unnecessary exception catches. Recently added related if removed from CppcheckSourceContainer.
- Condition moved from index.jelly to summary.jelly to be able to reuse the data and remove one unnecessary loading (performance).
- Construction "key = ++key" has no meaning, updated to "++key" to resolve a warning.
  • Loading branch information
mixalturek committed Feb 22, 2014
1 parent e912256 commit 5db2c791e4ceb93f5cafce525ddbde5047aa0b97
Showing 4 changed files with 61 additions and 67 deletions.
@@ -1,7 +1,6 @@
package org.jenkinsci.plugins.cppcheck;

import com.thalesgroup.hudson.plugins.cppcheck.CppcheckSource;
import com.thalesgroup.hudson.plugins.cppcheck.model.CppcheckFile;
import com.thalesgroup.hudson.plugins.cppcheck.model.CppcheckWorkspaceFile;

import hudson.XmlFile;
@@ -18,8 +17,8 @@
import java.io.File;
import java.io.IOException;
import java.io.Serializable;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;

/**
@@ -111,8 +110,7 @@ public CppcheckStatistics getStatistics() {
}

public CppcheckSourceContainer getCppcheckSourceContainer() {
lazyLoadSourceContainer();
return cppcheckSourceContainer;
return lazyLoadSourceContainer();
}

/**
@@ -307,26 +305,22 @@ private Object readResolve() {

/**
* Lazy load source container data if they are not already loaded.
*
* @return the loaded and parsed data or empty object on error
*/
private void lazyLoadSourceContainer() {
private CppcheckSourceContainer lazyLoadSourceContainer() {
// Backward compatibility with version 1.14 and less
if(cppcheckSourceContainer != null) {
return;
return cppcheckSourceContainer;
}

XmlFile xmlSourceContainer = new XmlFile(new File(owner.getRootDir(),
CppcheckPublisher.XML_FILE_DETAILS));
try {
cppcheckSourceContainer = (CppcheckSourceContainer) xmlSourceContainer.read();
return (CppcheckSourceContainer) xmlSourceContainer.read();
} catch (IOException e) {
try {
cppcheckSourceContainer = new CppcheckSourceContainer(null,
owner.getWorkspace(), owner.getModuleRoot(),
new ArrayList<CppcheckFile>());
} catch (IOException e1) {
// Ignore, there is nothing to do
} catch (InterruptedException e1) {
// Ignore, there is nothing to do
}
return new CppcheckSourceContainer(new HashMap<Integer,
CppcheckWorkspaceFile>());
}
}
}
@@ -34,7 +34,7 @@ public CppcheckSourceContainer(BuildListener listener,
cppcheckFile.setKey(key);
cppcheckWorkspaceFile.setCppcheckFile(cppcheckFile);
internalMap.put(key, cppcheckWorkspaceFile);
key = ++key;
++key;
}
}

@@ -55,10 +55,7 @@ private CppcheckWorkspaceFile getCppcheckWorkspaceFile(BuildListener listener,
CppcheckWorkspaceFile cppcheckWorkspaceFile = new CppcheckWorkspaceFile();
FilePath sourceFilePath = getSourceFile(workspace, scmRootDir, cppcheckFileName);
if (!sourceFilePath.exists()) {
if(listener != null) {
CppcheckLogger.log(listener, "[WARNING] - The source file '" + sourceFilePath.toURI() + "' doesn't exist on the slave. The ability to display its source code has been removed.");
}

CppcheckLogger.log(listener, "[WARNING] - The source file '" + sourceFilePath.toURI() + "' doesn't exist on the slave. The ability to display its source code has been removed.");
cppcheckWorkspaceFile.setFileName(null);
cppcheckWorkspaceFile.setSourceIgnored(true);
} else if (sourceFilePath.isDirectory()) {
@@ -57,10 +57,7 @@
</j:if>

<j:if test="${it.statistics.numberTotal != 0}">
<j:if test="${it.cppcheckSourceContainer != null}">
<h2>Details</h2>
<st:include page="summary.jelly"/>
</j:if>
<st:include page="summary.jelly"/>
</j:if>
</l:main-panel>
</l:layout>
@@ -1,45 +1,51 @@
<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:g="/jelly/cppcheck">
<st:header name="Content-Type" value="text/html;charset=UTF-8"/>
<table class="pane sortable" id="files">
<tr>
<td class="pane-header">Filename</td>
<td class="pane-header" style="width:2em">LineNumber</td>
<td class="pane-header" style="width:2em">CppCheckId</td>
<td class="pane-header" style="width:2em">Severity</td>
<td class="pane-header" style="width:60%">Message</td>
</tr>
<tbody>

<j:forEach var="elt" items="${it.cppcheckSourceContainer.internalMap.values()}">
<j:set var="cppcheckfile" value="${elt.cppcheckFile}"/>
<tr>
<td class="pane">
<j:if test="${elt.isSourceIgnored()}">
${cppcheckfile.fileName}
</j:if>
<j:if test="${not elt.isSourceIgnored()}">
<a href="source.${cppcheckfile.key}">${cppcheckfile.fileName}</a>
</j:if>


</td>
<td class="pane">
<j:if
test="${elt.isSourceIgnored()}">
${cppcheckfile.lineNumber}
</j:if>
<j:if test="${not elt.isSourceIgnored()}">
<a href="source.${cppcheckfile.key}#${cppcheckfile.lineNumber}">${cppcheckfile.lineNumber}
</a>
</j:if>
</td>
<g:format value="${cppcheckfile.cppCheckId}" severity="${cppcheckfile.severity}"/>
<g:format value="${cppcheckfile.severity}" severity="${cppcheckfile.severity}"/>
<g:format value="${cppcheckfile.message}" severity="${cppcheckfile.severity}"/>
</tr>
</j:forEach>


</tbody>
</table>

<j:set var="cachedContainer" value="${it.cppcheckSourceContainer}"/>

<j:if test="${cachedContainer != null}">

<h2>Details</h2>

<table class="pane sortable" id="files">
<tr>
<td class="pane-header">Filename</td>
<td class="pane-header" style="width:2em">LineNumber</td>
<td class="pane-header" style="width:2em">CppCheckId</td>
<td class="pane-header" style="width:2em">Severity</td>
<td class="pane-header" style="width:60%">Message</td>
</tr>
<tbody>

<j:forEach var="elt" items="${cachedContainer.internalMap.values()}">
<j:set var="cppcheckfile" value="${elt.cppcheckFile}"/>
<tr>
<td class="pane">
<j:if test="${elt.isSourceIgnored()}">
${cppcheckfile.fileName}
</j:if>
<j:if test="${not elt.isSourceIgnored()}">
<a href="source.${cppcheckfile.key}">${cppcheckfile.fileName}</a>
</j:if>
</td>
<td class="pane">
<j:if test="${elt.isSourceIgnored()}">
${cppcheckfile.lineNumber}
</j:if>
<j:if test="${not elt.isSourceIgnored()}">
<a href="source.${cppcheckfile.key}#${cppcheckfile.lineNumber}">${cppcheckfile.lineNumber}
</a>
</j:if>
</td>
<g:format value="${cppcheckfile.cppCheckId}" severity="${cppcheckfile.severity}"/>
<g:format value="${cppcheckfile.severity}" severity="${cppcheckfile.severity}"/>
<g:format value="${cppcheckfile.message}" severity="${cppcheckfile.severity}"/>
</tr>
</j:forEach>

</tbody>
</table>

</j:if>

</j:jelly>

0 comments on commit 5db2c79

Please sign in to comment.