Skip to content
Permalink
Browse files
Merge pull request #18 from miktap/master
[JENKINS-13109] Huge changelogs eat all the Java memory
  • Loading branch information
rpetti committed Apr 3, 2012
2 parents 11249ca + 5159c1c commit a2523100d02ca360fd23b382af4e531c4dc6ac59
Showing 6 changed files with 76 additions and 17 deletions.
@@ -48,6 +48,12 @@
*/
public class ChangelistBuilder implements Builder<Changelist> {
private final Logger logger = LoggerFactory.getLogger("perforce");
//Maximum amount of files to be recorded to a changelist
private int maxFiles;

public ChangelistBuilder(int maxFiles) {
this.maxFiles = maxFiles;
}

public String[] getBuildCmd(String p4exe, String id) {
return new String[] { p4exe, "describe", "-s", id };
@@ -152,8 +158,16 @@ public Changelist build(StringBuilder sb) throws PerforceException {
if(line.startsWith("Affected files")) {
logger.debug("reading files...");
List<Changelist.FileEntry> files = new ArrayList<Changelist.FileEntry>();
int fileCount = 0;

while(lines.hasMoreElements()) {
//Record a maximum of maxFiles files
if(maxFiles > 0) {
if(fileCount >= maxFiles) {
break;
}
fileCount++;
}
String entry = lines.nextToken();
logger.debug("File Line: " + entry);
// if(!entry.startsWith("..."))
@@ -59,12 +59,15 @@ public Changes(Depot depot) {
* Returns a single changelist specified by its number.
*
* @param number
* @param maxFiles
* The maximum number of affected files that will be recorded
* to a changelist. With negative value include all the files.
* @return
* @throws PerforceException
*/
public Changelist getChangelist(int number) throws PerforceException {
public Changelist getChangelist(int number, int maxFiles) throws PerforceException {

ChangelistBuilder builder = new ChangelistBuilder();
ChangelistBuilder builder = new ChangelistBuilder(maxFiles);
Changelist change = builder.build(getPerforceResponse(builder.getBuildCmd(getP4Exe(), Integer.toString(number))));
if(change == null)
throw new PerforceException("Failed to retrieve changelist " + number);
@@ -111,10 +114,12 @@ private String getWorkspacePathForFile(String file) throws PerforceException {
* The last changelist number to start from
* @param limit
* The maximum changes to return if less than 1, will return everything
* @param maxFiles
* The maximum amount of affected files in a changelist to be recorded
* @return
* @throws PerforceException
*/
public List<Changelist> getChangelists(String path, int lastChange, int limit) throws PerforceException {
public List<Changelist> getChangelists(String path, int lastChange, int limit, int maxFiles) throws PerforceException {
path = normalizePath(path);
if(lastChange > 0)
path += "@" + lastChange;
@@ -132,7 +137,7 @@ public List<Changelist> getChangelists(String path, int lastChange, int limit) t
List<Changelist> changes = new ArrayList<Changelist>();
for(String id : ids) {
try{
changes.add(getChangelist(new Integer(id)));
changes.add(getChangelist(new Integer(id), maxFiles));
} catch(Exception e){
throw new PerforceException("Could not retrieve changelists.\nResponse from perforce was:\n" + response, e);
}
@@ -416,13 +421,16 @@ private void addCommand(List<String> list, String... args) {
* Converts a list of numbers to a list of changes.
*
* @param numbers
* @param maxFiles
* The maximum number of affected files that will be recorded
* to a changelist. With negative value include all the files.
* @return
* @throws PerforceException
*/
public List<Changelist> getChangelistsFromNumbers(List<Integer> numbers) throws PerforceException {
public List<Changelist> getChangelistsFromNumbers(List<Integer> numbers, int maxFiles) throws PerforceException {
List<Changelist> changes = new ArrayList<Changelist>();
for(Integer id : numbers) {
changes.add(getChangelist(id));
changes.add(getChangelist(id, maxFiles));
}
return changes;
}
@@ -183,6 +183,11 @@ public class PerforceSCM extends SCM {
*/
int firstChange = -1;

/**
* Maximum amount of files that are recorded to a changelist, if < 1 show every file.
*/
int fileLimit = 0;

/**
* P4 user name(s) or regex user pattern to exclude from SCM poll to prevent build trigger.
* Multiple user names are deliminated by space.
@@ -274,6 +279,7 @@ public PerforceSCM(
boolean pollOnlyOnMaster,
String slaveClientNameFormat,
int firstChange,
int fileLimit,
PerforceRepositoryBrowser browser,
String excludedUsers,
String excludedFiles,
@@ -352,6 +358,7 @@ public PerforceSCM(
this.dontUpdateClient = dontUpdateClient;
this.slaveClientNameFormat = slaveClientNameFormat;
this.firstChange = firstChange;
this.fileLimit = fileLimit;
this.dontRenameClient = false;
this.useOldClientName = false;
this.p4Charset = Util.fixEmptyAndTrim(p4Charset);
@@ -809,7 +816,7 @@ public boolean checkout(AbstractBuild build, Launcher launcher,
} else {
changeNumbersTo = depot.getChanges().getChangeNumbersInRange(p4workspace, lastChange+1, newestChange);
}
changes = depot.getChanges().getChangelistsFromNumbers(changeNumbersTo);
changes = depot.getChanges().getChangelistsFromNumbers(changeNumbersTo, fileLimit);
}


@@ -1165,7 +1172,7 @@ private SCMRevisionState getCurrentDepotRevisionState(Workspace p4workspace, Abs
}
else {
for (int changeNumber : changeNumbers) {
if (isChangelistExcluded(depot.getChanges().getChangelist(changeNumber), project, logger)) {
if (isChangelistExcluded(depot.getChanges().getChangelist(changeNumber, fileLimit), project, logger)) {
logger.println("Changelist "+changeNumber+" is composed of file(s) and/or user(s) that are excluded.");
} else {
return new PerforceSCMRevisionState(changeNumber);
@@ -1899,7 +1906,7 @@ public FormValidation doCheckChangeList(StaplerRequest req) {
if (depot != null) {
try {
int number = Integer.parseInt(change);
Changelist changelist = depot.getChanges().getChangelist(number);
Changelist changelist = depot.getChanges().getChangelist(number, -1);
if (changelist.getChangeNumber() != number)
throw new PerforceException("broken");
} catch (Exception e) {
@@ -2707,6 +2714,24 @@ public String getFirstChange() {
return Integer.valueOf(firstChange).toString();
}

/**
* This is only for the config screen. Also, it returns a string and not an int.
* This is because we want to show an empty value in the config option if it is not being
* used. The default value of -1 is not exactly empty. So if we are set to default of
* -1, we return an empty string. Anything else and we return the actual change number.
*
* @return fileLimit
*/
public String getFileLimit() {
if (fileLimit <= 0)
return "";
return Integer.valueOf(fileLimit).toString();
}

public void setFileLimit(int fileLimit) {
this.fileLimit = fileLimit;
}

/**
* With Perforce the server keeps track of files in the workspace. We never
* want files deleted without the knowledge of the server so we disable the
@@ -121,6 +121,10 @@
checkUrl="'${rootURL}/scm/PerforceSCM/checkChangeList?port='+escape(this.form.elements['p4Port'].value)+'&amp;tool='+escape(this.form.elements['p4Tool'].value)+'&amp;user='+escape(this.form.elements['p4User'].value)+'&amp;pass='+escape(this.form.elements['p4Passwd'].value)+'&amp;change='+escape(this.value)"/>
</f:entry>

<f:entry title="Changelist file limit" help="/plugin/perforce/help/fileLimit.html">
<f:textbox field="fileLimit"/>
</f:entry>

<f:entry title="Client Line Endings" help="/plugin/perforce/help/lineEnd.html">
<select name="lineEndValue" id="lineEndValue">
<j:forEach var="value" items="${instance==null?descriptor['allLineEndChoices']:instance['allLineEndChoices']}">
@@ -0,0 +1,8 @@
<div>
<p>
The maximum amount of affected files in a changelist that will be recorded to the detailed changes view. Empty value or 0 or negative value will list all the files in a changelist.
</p>
<p>
Limiting the amount of files can prevent out of memory errors.
</p>
</div>
@@ -27,7 +27,7 @@ public void testConfigRoundtrip() throws Exception {
PerforceSCM scm = new PerforceSCM(
"user", "pass", "client", "port", "", "exe", "sysRoot",
"sysDrive", "label", "counter", "shared", "charset", "charset2", false, true, true, true, true, true, false,
false, true, false, false, false, false, "${basename}", 0, browser, "exclude_user", "exclude_file", true);
false, true, false, false, false, false, "${basename}", 0, -1, browser, "exclude_user", "exclude_file", true);
scm.setProjectPath("path");
project.setScm(scm);

@@ -36,7 +36,7 @@ public void testConfigRoundtrip() throws Exception {

// verify that the data is intact
assertEqualBeans(scm, project.getScm(),
"p4User,p4Client,p4Port,p4Label,projectPath,p4Exe,p4SysRoot,p4SysDrive,forceSync,alwaysForceSync,dontUpdateClient,createWorkspace,updateView,slaveClientNameFormat,lineEndValue,firstChange,p4Counter,updateCounterValue,exposeP4Passwd,useViewMaskForPolling,viewMask,useViewMaskForSyncing,p4Charset,p4CommandCharset,p4Stream,useStreamDepot");
"p4User,p4Client,p4Port,p4Label,projectPath,p4Exe,p4SysRoot,p4SysDrive,forceSync,alwaysForceSync,dontUpdateClient,createWorkspace,updateView,slaveClientNameFormat,lineEndValue,firstChange,p4Counter,updateCounterValue,exposeP4Passwd,useViewMaskForPolling,viewMask,useViewMaskForSyncing,p4Charset,p4CommandCharset,p4Stream,useStreamDepot,fileLimit");
assertEquals("exclude_user", scm.getExcludedUsers());
assertEquals("exclude_file", scm.getExcludedFiles());
//assertEqualBeans(scm.getBrowser(),p.getScm().getBrowser(),"URL");
@@ -48,7 +48,7 @@ public void testConfigRoundtripWithStream() throws Exception {
PerforceSCM scm = new PerforceSCM(
"user", "pass", "client", "port", "", "exe", "sysRoot",
"sysDrive", "label", "counter", "shared", "charset", "charset2", false, true, true, true, true, true, false,
false, true, false, false, false, false, "${basename}", 0, browser, "exclude_user", "exclude_file", true);
false, true, false, false, false, false, "${basename}", 0, -1, browser, "exclude_user", "exclude_file", true);
scm.setP4Stream("stream");
scm.setUseStreamDepot(true);
project.setScm(scm);
@@ -75,7 +75,7 @@ public void testConfigPasswordEnctyptionAndDecription() throws Exception {
PerforceSCM scm = new PerforceSCM(
"user", password, "client", "port", "", "test_installation", "sysRoot",
"sysDrive", "label", "counter", "shared", "charset", "charset2", false, true, true, true, true, true, false,
false, true, false, false, false, false, "${basename}", 0, browser, "exclude_user", "exclude_file", true);
false, true, false, false, false, false, "${basename}", 0, -1, browser, "exclude_user", "exclude_file", true);
scm.setProjectPath("path");
project.setScm(scm);

@@ -84,7 +84,7 @@ public void testConfigPasswordEnctyptionAndDecription() throws Exception {

// verify that the data is intact
assertEqualBeans(scm, project.getScm(),
"p4User,p4Client,p4Port,p4Label,projectPath,p4Tool,p4SysRoot,p4SysDrive,forceSync,alwaysForceSync,dontUpdateClient,updateView,slaveClientNameFormat,lineEndValue,firstChange,p4Counter,updateCounterValue,exposeP4Passwd,useViewMaskForPolling,viewMask,useViewMaskForSyncing,p4Charset,p4CommandCharset,p4Stream,useStreamDepot");
"p4User,p4Client,p4Port,p4Label,projectPath,p4Tool,p4SysRoot,p4SysDrive,forceSync,alwaysForceSync,dontUpdateClient,updateView,slaveClientNameFormat,lineEndValue,firstChange,p4Counter,updateCounterValue,exposeP4Passwd,useViewMaskForPolling,viewMask,useViewMaskForSyncing,p4Charset,p4CommandCharset,p4Stream,useStreamDepot,fileLimit");
assertEquals("exclude_user", scm.getExcludedUsers());
assertEquals("exclude_file", scm.getExcludedFiles());

@@ -104,7 +104,7 @@ public void testDepotContainsUnencryptedPassword() throws Exception {
PerforceSCM scm = new PerforceSCM(
"user", password, "client", "port", "", "test_installation", "sysRoot",
"sysDrive", "label", "counter", "shared", "charset", "charset2", false, true, true, true, true, true, false,
false, true, false, false, false, false, "${basename}", 0, browser, "exclude_user", "exclude_file", true);
false, true, false, false, false, false, "${basename}", 0, -1, browser, "exclude_user", "exclude_file", true);
scm.setProjectPath("path");
project.setScm(scm);

@@ -122,7 +122,7 @@ public void testConfigSaveReloadAndSaveDoesNotDoubleEncryptThePassword() throws
PerforceSCM scm = new PerforceSCM(
"user", password, "client", "port", "", "test_installation", "sysRoot",
"sysDrive", "label", "counter", "shared", "charset", "charset2", false, true, true, true, true, true, false,
false, true, false, false, false, false, "${basename}", 0, browser, "exclude_user", "exclude_file", true);
false, true, false, false, false, false, "${basename}", 0, -1, browser, "exclude_user", "exclude_file", true);
scm.setProjectPath("path");
project.setScm(scm);

@@ -132,7 +132,7 @@ public void testConfigSaveReloadAndSaveDoesNotDoubleEncryptThePassword() throws

// verify that the data is intact
assertEqualBeans(scm, project.getScm(),
"p4User,p4Client,p4Port,p4Label,projectPath,p4Tool,p4SysRoot,p4SysDrive,forceSync,alwaysForceSync,dontUpdateClient,updateView,slaveClientNameFormat,lineEndValue,firstChange,p4Counter,updateCounterValue,exposeP4Passwd,useViewMaskForPolling,viewMask,useViewMaskForSyncing,p4Charset,p4CommandCharset,p4Stream,useStreamDepot");
"p4User,p4Client,p4Port,p4Label,projectPath,p4Tool,p4SysRoot,p4SysDrive,forceSync,alwaysForceSync,dontUpdateClient,updateView,slaveClientNameFormat,lineEndValue,firstChange,p4Counter,updateCounterValue,exposeP4Passwd,useViewMaskForPolling,viewMask,useViewMaskForSyncing,p4Charset,p4CommandCharset,p4Stream,useStreamDepot,fileLimit");
assertEquals("exclude_user", scm.getExcludedUsers());
assertEquals("exclude_file", scm.getExcludedFiles());

0 comments on commit a252310

Please sign in to comment.