Skip to content

Commit

Permalink
[JENKINS-13814] don't buffer the entire rlog output in memory.
Browse files Browse the repository at this point in the history
The size of the data "cvs rlog" produces is roughtly O(N*M) where
N is the # of files in the directory and M is the amount of changes.

So even when a delta is small, on a large repository this can produce
significant amount of data. Use of ByteArrayOutputStream causes a large
spike that can kill a VM, so spill the data over to disk if we are
getting large output.
  • Loading branch information
kohsuke committed Jun 28, 2012
1 parent 5cc27a9 commit 2565a8f
Show file tree
Hide file tree
Showing 4 changed files with 388 additions and 303 deletions.
49 changes: 49 additions & 0 deletions src/main/java/hudson/scm/CVSChangeLogSet.java
Expand Up @@ -29,7 +29,10 @@
import hudson.util.Digester2; import hudson.util.Digester2;
import hudson.util.IOException2; import hudson.util.IOException2;


import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException; import java.io.IOException;
import java.io.PrintStream;
import java.text.DateFormat; import java.text.DateFormat;
import java.text.ParseException; import java.text.ParseException;
import java.text.SimpleDateFormat; import java.text.SimpleDateFormat;
Expand Down Expand Up @@ -683,4 +686,50 @@ public String toString() {
return buf.toString(); return buf.toString();
} }
} }

public void toFile(final java.io.File changelogFile) throws IOException {
PrintStream output = new PrintStream(new FileOutputStream(changelogFile));

DateFormat format = new SimpleDateFormat("yyyy/MM/dd HH:mm:ss");

output.println("<?xml version=\"1.0\" encoding=\"UTF-8\"?>");
output.println("<changelog>");

for (CVSChangeLog entry : this) {

output.println("\t<entry>");
output.println("\t\t<changeDate>" + format.format(entry.getChangeDate()) + "</changeDate>");
output.println("\t\t<author><![CDATA[" + entry.getAuthor() + "]]></author>");

for (CVSChangeLogSet.File file : entry.getFiles()) {

output.println("\t\t<file>");
output.println("\t\t\t<name><![CDATA[" + file.getName() + "]]></name>");

if (file.getFullName() != null) {
output.println("\t\t\t<fullName><![CDATA[" + file.getFullName() + "]]></fullName>");
}

output.println("\t\t\t<revision>" + file.getRevision() + "</revision>");

final String previousRevision = file.getPrevrevision();

if (previousRevision != null) {
output.println("\t\t\t<prevrevision>" + previousRevision + "</prevrevision>");
}

if (file.isDead()) {
output.println("\t\t\t<dead />");
}

output.println("\t\t</file>");
}

output.println("\t\t<msg><![CDATA[" + entry.getMsg() + "]]></msg>");
output.println("\t</entry>");
}
output.println("</changelog>");
output.flush();
output.close();
}
} }
41 changes: 30 additions & 11 deletions src/main/java/hudson/scm/CVSSCM.java
Expand Up @@ -44,10 +44,13 @@
import hudson.scm.cvstagging.LegacyTagAction; import hudson.scm.cvstagging.LegacyTagAction;
import hudson.util.Secret; import hudson.util.Secret;


import java.io.ByteArrayOutputStream; import java.io.ByteArrayInputStream;
import java.io.File; import java.io.File;
import java.io.FileReader;
import java.io.IOException; import java.io.IOException;
import java.io.InputStreamReader;
import java.io.PrintStream; import java.io.PrintStream;
import java.io.Reader;
import java.io.Serializable; import java.io.Serializable;
import java.lang.reflect.Field; import java.lang.reflect.Field;
import java.text.DateFormat; import java.text.DateFormat;
Expand All @@ -67,6 +70,7 @@


import net.sf.json.JSONObject; import net.sf.json.JSONObject;


import org.apache.commons.io.output.DeferredFileOutputStream;
import org.kohsuke.stapler.DataBoundConstructor; import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.StaplerRequest; import org.kohsuke.stapler.StaplerRequest;
import org.kohsuke.stapler.export.Exported; import org.kohsuke.stapler.export.Exported;
Expand Down Expand Up @@ -412,11 +416,11 @@ private List<CVSChangeLog> calculateChangeLog(final Date startTime, final Date e
for (final CvsRepositoryItem item : repository.getRepositoryItems()) { for (final CvsRepositoryItem item : repository.getRepositoryItems()) {
for (final CvsModule module : item.getModules()) { for (final CvsModule module : item.getModules()) {


String logContents = getRemoteLogForModule(repository, item, module, listener.getLogger(), startTime, endTime, envVars); CvsLog logContents = getRemoteLogForModule(repository, item, module, listener.getLogger(), startTime, endTime, envVars);


// use the parser to build up a list of changes and add it to the // use the parser to build up a list of changes and add it to the
// list we've been creating // list we've been creating
changes.addAll(CvsChangeLogHelper.getInstance().mapCvsLog(logContents, repository, item, module, envVars).getChanges()); changes.addAll(logContents.mapCvsLog(repository, item, module, envVars).getChanges());
} }
} }
return changes; return changes;
Expand Down Expand Up @@ -458,11 +462,11 @@ private List<CvsFile> calculateRepositoryState(final Date startTime, final Date
for (final CvsRepositoryItem item : repository.getRepositoryItems()) { for (final CvsRepositoryItem item : repository.getRepositoryItems()) {
for (final CvsModule module : item.getModules()) { for (final CvsModule module : item.getModules()) {


String logContents = getRemoteLogForModule(repository, item, module, listener.getLogger(), startTime, endTime, envVars); CvsLog logContents = getRemoteLogForModule(repository, item, module, listener.getLogger(), startTime, endTime, envVars);


// use the parser to build up a list of changed files and add it to // use the parser to build up a list of changed files and add it to
// the list we've been creating // the list we've been creating
files.addAll(CvsChangeLogHelper.getInstance().mapCvsLog(logContents, repository, item, module, envVars).getFiles()); files.addAll(logContents.mapCvsLog(repository, item, module, envVars).getFiles());


} }
} }
Expand All @@ -487,7 +491,7 @@ private List<CvsFile> calculateRepositoryState(final Date startTime, final Date
* @throws IOException * @throws IOException
* on underlying communication failure * on underlying communication failure
*/ */
private String getRemoteLogForModule(final CvsRepository repository, final CvsRepositoryItem item, final CvsModule module, private CvsLog getRemoteLogForModule(final CvsRepository repository, final CvsRepositoryItem item, final CvsModule module,
final PrintStream errorStream, final Date startTime, final Date endTime, final EnvVars envVars) throws IOException { final PrintStream errorStream, final Date startTime, final Date endTime, final EnvVars envVars) throws IOException {
final Client cvsClient = getCvsClient(repository, envVars); final Client cvsClient = getCvsClient(repository, envVars);


Expand All @@ -509,7 +513,8 @@ private String getRemoteLogForModule(final CvsRepository repository, final CvsRe


// create an output stream to send the output from CVS command to - we // create an output stream to send the output from CVS command to - we
// can then parse it from here // can then parse it from here
final ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); final File tmpRlogSpill = File.createTempFile("cvs","rlog");
final DeferredFileOutputStream outputStream = new DeferredFileOutputStream(100*1024,tmpRlogSpill);
final PrintStream logStream = new PrintStream(outputStream); final PrintStream logStream = new PrintStream(outputStream);


// set a listener with our output stream that we parse the log from // set a listener with our output stream that we parse the log from
Expand Down Expand Up @@ -539,11 +544,25 @@ private String getRemoteLogForModule(final CvsRepository repository, final CvsRe
} }


// flush the output so we have it all available for parsing // flush the output so we have it all available for parsing
logStream.flush(); logStream.close();
outputStream.flush();


// return the contents of the stream as the output of the command // return the contents of the stream as the output of the command
return outputStream.toString(); return new CvsLog() {
@Override
public Reader read() throws IOException {
// TODO: is it really correct that we read this in the platform encoding?
// note that master and slave can have different platform encoding
if (outputStream.isInMemory())
return new InputStreamReader(new ByteArrayInputStream(outputStream.getData()));
else
return new FileReader(outputStream.getFile());
}

@Override
public void dispose() {
tmpRlogSpill.delete();
}
};
} }


/** /**
Expand Down Expand Up @@ -834,7 +853,7 @@ public boolean checkout(final AbstractBuild<?, ?> build, final Launcher launcher
changes.addAll(calculateChangeLog(lastCompleteBuild.getTime(), build.getTime(), location, launcher, changes.addAll(calculateChangeLog(lastCompleteBuild.getTime(), build.getTime(), location, launcher,
workspace, listener, build.getEnvironment(listener))); workspace, listener, build.getEnvironment(listener)));
} }
CvsChangeLogHelper.getInstance().toFile(changes, changelogFile); new CVSChangeLogSet(build,changes).toFile(changelogFile);
} }


// add the current workspace state as an action // add the current workspace state as an action
Expand Down

0 comments on commit 2565a8f

Please sign in to comment.