From f04bd57f837ba147fc3ad4fcedd60ad8c481a12d Mon Sep 17 00:00:00 2001 From: Andrew Gvozdev Date: Mon, 14 Nov 2011 13:12:38 -0500 Subject: [PATCH] bug 259768: "Use optimal jobs number" highly misleading. Builder.getParallelizationNum() to return always positive value --- .../core/IMultiConfiguration.java | 27 ++- .../core/InternalBuildRunner.java | 2 +- .../internal/buildmodel/ParallelBuilder.java | 2 +- .../managedbuilder/internal/core/Builder.java | 106 +++++----- .../internal/core/Configuration.java | 10 +- .../newmake/core/IMakeCommonBuildInfo.java | 18 +- .../ui/properties/BuildBehaviourTab.java | 186 ++++++++++-------- .../ui/properties/BuilderSettingsTab.java | 2 + 8 files changed, 176 insertions(+), 177 deletions(-) diff --git a/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/core/IMultiConfiguration.java b/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/core/IMultiConfiguration.java index f1307bb6bac..98498c8cb4d 100644 --- a/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/core/IMultiConfiguration.java +++ b/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/core/IMultiConfiguration.java @@ -14,7 +14,6 @@ import org.eclipse.cdt.core.settings.model.ICMultiItemsHolder; import org.eclipse.cdt.managedbuilder.buildproperties.IBuildProperty; import org.eclipse.cdt.managedbuilder.buildproperties.IBuildPropertyValue; import org.eclipse.cdt.managedbuilder.internal.core.Builder; -import org.eclipse.cdt.newmake.core.IMakeCommonBuildInfo; /** * This class is to combine multiple configurations to one to support @@ -26,12 +25,13 @@ import org.eclipse.cdt.newmake.core.IMakeCommonBuildInfo; public interface IMultiConfiguration extends IConfiguration, ICMultiItemsHolder { /** - * Check if the configuration's builder is operating in parallel mode. - * @return {@code true} if parallel mode is enabled, {@code false} otherwise. + * Check if all configurations' builders are operating in parallel mode. + * @return {@code true} if parallel mode is enabled for all configurations, + * {@code false} otherwise. */ boolean getParallelDef(); /** - * Set parallel execution mode for the configuration's builder. + * Set same parallel execution mode for all configurations' builders. * @see Builder#setParallelBuildOn(boolean) * * @param parallel - the flag to enable or disable parallel mode. @@ -39,18 +39,16 @@ public interface IMultiConfiguration extends IConfiguration, ICMultiItemsHolder void setParallelDef(boolean parallel); /** - * Returns maximum number of parallel threads/jobs used by the configuration's builder. - * Note that this function can return negative value to indicate "optimal" number. - * + * Returns maximum number of parallel threads/jobs used by the configurations' builders. * @see #setParallelDef(boolean) - * @see Builder#getParallelizationNum() * - * @return - maximum number of parallel threads or jobs used by the builder or negative number. - * For exact interpretation see table in {@link IMakeCommonBuildInfo#getParallelizationNum()} + * @return - maximum number of parallel threads or jobs used by each builder or 0 if the numbers + * don't match. */ int getParallelNumber(); + /** - * Sets maximum number of parallel threads/jobs to be used by builder. + * Sets maximum number of parallel threads/jobs to be used by each builder. * Note that the number will be set only if the builder is in "parallel" * mode. * @@ -61,10 +59,9 @@ public interface IMultiConfiguration extends IConfiguration, ICMultiItemsHolder void setParallelNumber(int jobs); /** - * returns the Internal Builder parallel mode - * if true, internal builder will work in parallel mode - * otherwise it will use only one thread - * @return boolean + * Check if all configurations' internal builders are operating in parallel mode. + * @return {@code true} if parallel mode is enabled for all configurations, + * {@code false} otherwise. * * @deprecated since CDT 9.0. Use {@link #getParallelDef()} */ diff --git a/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/core/InternalBuildRunner.java b/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/core/InternalBuildRunner.java index 502c13732da..8c2cf096a4f 100644 --- a/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/core/InternalBuildRunner.java +++ b/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/core/InternalBuildRunner.java @@ -65,7 +65,7 @@ public class InternalBuildRunner extends AbstractBuildRunner { public boolean invokeBuild(int kind, IProject project, IConfiguration configuration, IBuilder builder, IConsole console, IMarkerGenerator markerGenerator, IncrementalProjectBuilder projectBuilder, IProgressMonitor monitor) throws CoreException { - boolean isParallel = Math.abs(builder.getParallelizationNum()) > 1; + boolean isParallel = builder.getParallelizationNum() > 1; // boolean buildIncrementaly = true; boolean resumeOnErr = !builder.isStopOnError(); diff --git a/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/internal/buildmodel/ParallelBuilder.java b/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/internal/buildmodel/ParallelBuilder.java index 7dfcedf63c4..814e0310bf8 100644 --- a/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/internal/buildmodel/ParallelBuilder.java +++ b/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/internal/buildmodel/ParallelBuilder.java @@ -206,7 +206,7 @@ public class ParallelBuilder { if(cwd == null) cwd = des.getDefaultBuildDirLocation(); int threads = 1; if (cfg instanceof Configuration) { - threads = Math.abs(((Configuration)cfg).getParallelNumber()); + threads = ((Configuration)cfg).getParallelNumber(); } ParallelBuilder builder = new ParallelBuilder(cwd, dirs, out, err, monitor, resumeOnErrors, buildIncrementally); builder.enqueueAll(des); diff --git a/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/internal/core/Builder.java b/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/internal/core/Builder.java index 22172cf3b8a..b9e27e02f97 100644 --- a/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/internal/core/Builder.java +++ b/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/internal/core/Builder.java @@ -67,6 +67,7 @@ import org.eclipse.cdt.managedbuilder.makegen.gnu.GnuMakefileGenerator; import org.eclipse.cdt.newmake.core.IMakeCommonBuildInfo; import org.eclipse.core.resources.IProject; import org.eclipse.core.resources.IncrementalProjectBuilder; +import org.eclipse.core.runtime.Assert; import org.eclipse.core.runtime.CoreException; import org.eclipse.core.runtime.IConfigurationElement; import org.eclipse.core.runtime.IExtension; @@ -123,7 +124,7 @@ public class Builder extends HoldsOptions implements IBuilder, IMatchKeyProvider // parallelization private String parallelBuildCmd; private Boolean isParallelBuildEnabled; - private Integer parallelJobsNumber; // negative number denotes "optimal" value, see getOptimalParallelJobNum() + private Integer parallelNumberAttribute; // negative number denotes "optimal" value, see getOptimalParallelJobNum() private boolean isTest; @@ -331,7 +332,7 @@ public class Builder extends HoldsOptions implements IBuilder, IMatchKeyProvider ignoreErrCmd = builder.ignoreErrCmd; isParallelBuildEnabled = builder.isParallelBuildEnabled; - parallelJobsNumber = builder.parallelJobsNumber; + parallelNumberAttribute = builder.parallelNumberAttribute; parallelBuildCmd = builder.parallelBuildCmd; if(builder.outputEntries != null){ @@ -381,9 +382,9 @@ public class Builder extends HoldsOptions implements IBuilder, IMatchKeyProvider } catch (CoreException e) { } } - if (getParallelizationNum() != builder.getParallelizationNum() && supportsParallelBuild()) { + if (getParallelizationNumAttribute() != builder.getParallelizationNumAttribute() && supportsParallelBuild()) { try { - setParallelizationNum(builder.getParallelizationNum()); + setParallelizationNum(builder.getParallelizationNumAttribute()); } catch (CoreException e) { } } @@ -631,29 +632,29 @@ public class Builder extends HoldsOptions implements IBuilder, IMatchKeyProvider private void decodeParallelizationNumber(String value) { if (VALUE_OPTIMAL.equals(value)) { - parallelJobsNumber = -getOptimalParallelJobNum(); + parallelNumberAttribute = -getOptimalParallelJobNum(); } else if (VALUE_UNLIMITED.equals(value)) { if (!isInternalBuilder()) { - parallelJobsNumber = Integer.MAX_VALUE; + parallelNumberAttribute = Integer.MAX_VALUE; } else { ManagedBuilderCorePlugin.error("'unlimited' number of jobs is not allowed for Internal Builder, switching to 'optimal'"); - parallelJobsNumber = -getOptimalParallelJobNum(); + parallelNumberAttribute = -getOptimalParallelJobNum(); } } else { try { - parallelJobsNumber = Integer.decode(value); + parallelNumberAttribute = Integer.decode(value); } catch (NumberFormatException e) { ManagedBuilderCorePlugin.log(e); - parallelJobsNumber = getOptimalParallelJobNum(); + parallelNumberAttribute = getOptimalParallelJobNum(); } - if (parallelJobsNumber <= 0) { + if (parallelNumberAttribute <= 0) { // compatibility with legacy representation - it was that inconsistent if (isInternalBuilder()) { // "optimal" for Internal Builder - parallelJobsNumber = -getOptimalParallelJobNum(); + parallelNumberAttribute = -getOptimalParallelJobNum(); } else { // unlimited for External Builder - parallelJobsNumber = Integer.MAX_VALUE; + parallelNumberAttribute = Integer.MAX_VALUE; } } } @@ -918,8 +919,8 @@ public class Builder extends HoldsOptions implements IBuilder, IMatchKeyProvider if (isParallelBuildEnabled != null) element.setAttribute(ATTRIBUTE_PARALLEL_BUILD_ON, isParallelBuildEnabled.toString()); - if (isParallelBuildOn() && parallelJobsNumber != null) - element.setAttribute(ATTRIBUTE_PARALLELIZATION_NUMBER, encodeParallelizationNumber(parallelJobsNumber)); + if (isParallelBuildOn() && parallelNumberAttribute != null) + element.setAttribute(ATTRIBUTE_PARALLELIZATION_NUMBER, encodeParallelizationNumber(parallelNumberAttribute)); // Note: build file generator cannot be specified in a project file because // an IConfigurationElement is needed to load it! @@ -1016,8 +1017,8 @@ public class Builder extends HoldsOptions implements IBuilder, IMatchKeyProvider if (isParallelBuildEnabled != null) element.setAttribute(ATTRIBUTE_PARALLEL_BUILD_ON, isParallelBuildEnabled.toString()); - if (isParallelBuildOn() && parallelJobsNumber != null) - element.setAttribute(ATTRIBUTE_PARALLELIZATION_NUMBER, encodeParallelizationNumber(parallelJobsNumber)); + if (isParallelBuildOn() && parallelNumberAttribute != null) + element.setAttribute(ATTRIBUTE_PARALLELIZATION_NUMBER, encodeParallelizationNumber(parallelNumberAttribute)); // Note: build file generator cannot be specified in a project file because // an IConfigurationElement is needed to load it! @@ -1095,10 +1096,11 @@ public class Builder extends HoldsOptions implements IBuilder, IMatchKeyProvider public String getArguments() { String args = getArgumentsAttribute(); String stopOnErrCmd = getStopOnErrCmd(isStopOnError()); - String parallelCmd = isParallelBuildOn() ? getParallelizationCmd(getParallelizationNum()) : EMPTY_STRING; + int parallelNum = getParallelizationNum(); + String parallelCmd = isParallelBuildOn() ? getParallelizationCmd(parallelNum) : EMPTY_STRING; String reversedStopOnErrCmd = getStopOnErrCmd(!isStopOnError()); - String reversedParallelBuildCmd = !isParallelBuildOn() ? getParallelizationCmd(getParallelizationNum()) : EMPTY_STRING; + String reversedParallelBuildCmd = !isParallelBuildOn() ? getParallelizationCmd(parallelNum) : EMPTY_STRING; args = removeCmd(args, reversedStopOnErrCmd); args = removeCmd(args, reversedParallelBuildCmd); @@ -1167,16 +1169,14 @@ public class Builder extends HoldsOptions implements IBuilder, IMatchKeyProvider return index; } - public String getParallelizationCmd(int num){ + public String getParallelizationCmd(int num) { String pattern = getParrallelBuildCmd(); - if(pattern.length() == 0){ - return EMPTY_STRING; - }if(num == 0){ + if (pattern.length() == 0 || num == 0) { return EMPTY_STRING; } // "unlimited" number of jobs results in not adding the number to parallelization cmd // that behavior corresponds that of "make" flag "-j". - return processParallelPattern(pattern, num == Integer.MAX_VALUE, Math.abs(num)); + return processParallelPattern(pattern, num == Integer.MAX_VALUE, num); } /** @@ -1188,6 +1188,8 @@ public class Builder extends HoldsOptions implements IBuilder, IMatchKeyProvider *
Where # is num or empty if {@code empty} is {@code true}) */ private String processParallelPattern(String pattern, boolean empty, int num){ + Assert.isTrue(num > 0); + int start = pattern.indexOf(PARALLEL_PATTERN_NUM_START); int end = -1; boolean hasStartChar = false; @@ -2527,46 +2529,40 @@ public class Builder extends HoldsOptions implements IBuilder, IMatchKeyProvider } /** - * {@inheritDoc} - * - * Returns the number of parallel jobs to be used for a build. + * Returns the internal representation of maximum number of parallel jobs + * to be used for a build. * Note that "optimal" value is represented by negative number. - * See the table at {@link IMakeCommonBuildInfo#getParallelizationNum()}. + * + * The value of the number is encoded as follows: + * 
+	 *  Status       Returns
+	 * No parallel      1       
+	 * Optimal       -CPU#       (negative number of processors) 
+	 * Specific        >0        (positive number)
+	 * Unlimited    Integer.MAX  (N/A for Internal Builder)
+	 *
*/ - @Override - public int getParallelizationNum() { + public int getParallelizationNumAttribute() { if (!isParallelBuildOn()) return 1; - - if(parallelJobsNumber == null){ - if(superClass != null){ - return ((Builder)superClass).getParallelizationNum(); - } - return 1; - } - return parallelJobsNumber.intValue(); - } - - public int getParallelizationNumAttribute(){ - if(parallelJobsNumber == null){ + + if(parallelNumberAttribute == null){ if(superClass != null){ return ((Builder)superClass).getParallelizationNumAttribute(); } return 1; } - return parallelJobsNumber.intValue(); + return parallelNumberAttribute.intValue(); + } + + @Override + public int getParallelizationNum() { + return Math.abs(getParallelizationNumAttribute()); } - /** - * {@inheritDoc} - * - * @param jobs - maximum number of jobs or threads. If the number is 0 - * or negative, negative "optimal" number will be set, see - * {@link #getOptimalParallelJobNum()}. - */ @Override public void setParallelizationNum(int jobs) throws CoreException { - if (isParallelBuildOn() && (parallelJobsNumber == null || parallelJobsNumber != jobs)) { + if (isParallelBuildOn() && (parallelNumberAttribute == null || parallelNumberAttribute != jobs)) { String curCmd = getParallelizationCmd(getParallelizationNum()); String args = getArgumentsAttribute(); String updatedArgs = removeCmd(args, curCmd); @@ -2579,10 +2575,10 @@ public class Builder extends HoldsOptions implements IBuilder, IMatchKeyProvider jobs = -1; } if (jobs > 0) { - parallelJobsNumber = jobs; + parallelNumberAttribute = jobs; } else { // "optimal" - parallelJobsNumber = -getOptimalParallelJobNum(); + parallelNumberAttribute = -getOptimalParallelJobNum(); } setDirty(true); } @@ -2647,7 +2643,7 @@ public class Builder extends HoldsOptions implements IBuilder, IMatchKeyProvider * * @param on - the flag to enable or disable parallel mode. *
{@code true} to enable, in this case the maximum number of jobs - * will be set to negative "optimal" number, see {@link #getOptimalParallelJobNum()}. + * will be set to "optimal" number, see {@link #getOptimalParallelJobNum()}. *
{@code false} to disable, the number of jobs will be set to 1. */ @Override @@ -2668,9 +2664,9 @@ public class Builder extends HoldsOptions implements IBuilder, IMatchKeyProvider if (isParallelBuildEnabled) { // "optimal" - parallelJobsNumber = -getOptimalParallelJobNum(); + parallelNumberAttribute = -getOptimalParallelJobNum(); } else { - parallelJobsNumber = 1; + parallelNumberAttribute = 1; } setDirty(true); } diff --git a/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/internal/core/Configuration.java b/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/internal/core/Configuration.java index 271b1c94aef..5d552fc9594 100644 --- a/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/internal/core/Configuration.java +++ b/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/internal/core/Configuration.java @@ -2273,8 +2273,8 @@ public class Configuration extends BuildObject implements IConfiguration, IBuild * mode. * * @param jobs - maximum number of jobs or threads. If the number is 0 - * or negative, negative "optimal" number will be set, see - * {@link Builder#getOptimalParallelJobNum()}. + * or negative, "optimal" number will be set, + * see {@link Builder#getOptimalParallelJobNum()}. */ public void setParallelNumber(int jobs){ try { @@ -2286,13 +2286,9 @@ public class Configuration extends BuildObject implements IConfiguration, IBuild /** * Returns maximum number of parallel threads/jobs used by the configuration's builder. - * Note that this function can return negative value to indicate "optimal" number. - * * @see #setParallelDef(boolean) - * @see Builder#getParallelizationNum() * - * @return - maximum number of parallel threads or jobs used by the builder or negative number. - * For exact interpretation see table in {@link IMakeCommonBuildInfo#getParallelizationNum()} + * @return - maximum number of parallel threads or jobs used by the builder. */ public int getParallelNumber(){ return getBuilder().getParallelizationNum(); diff --git a/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/newmake/core/IMakeCommonBuildInfo.java b/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/newmake/core/IMakeCommonBuildInfo.java index 3d1a919d7fa..553c78170b9 100644 --- a/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/newmake/core/IMakeCommonBuildInfo.java +++ b/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/newmake/core/IMakeCommonBuildInfo.java @@ -44,15 +44,7 @@ public interface IMakeCommonBuildInfo { boolean supportsStopOnError(boolean on); /** - * @return the current number of parallel jobs. - * The value of the number is encoded as follows: - * 
-	 *  Status       Returns
-	 * No parallel      1       
-	 * Unlimited    Integer.MAX  (N/A for Internal Builder)
-	 * Optimal       -CPU#       (negative number of processors) 
-	 * Specific        >0        (positive number)
-	 *
+ * @return the maximum number of parallel jobs to be used for build. */ int getParallelizationNum(); @@ -61,21 +53,25 @@ public interface IMakeCommonBuildInfo { * Note that the number will be set only if the builder is in "parallel" * mode. * - * @param jobs - number of jobs according table {@link #getParallelizationNum()}. + * @param jobs - maximum number of jobs. * Any number <=0 is treated as setting "optimal" property, - * the value of the number itself is ignored. + * the value of the number itself is ignored in this case. */ void setParallelizationNum(int jobs) throws CoreException; + /** * @return {@code true} if builder supports parallel build, * {@code false} otherwise. */ + boolean supportsParallelBuild(); + /** * @return {@code true} if builder support for parallel build is enabled, * {@code false} otherwise. */ boolean isParallelBuildOn(); + /** * Set parallel execution mode for the builder. * @see Builder#setParallelBuildOn(boolean) diff --git a/build/org.eclipse.cdt.managedbuilder.ui/src/org/eclipse/cdt/managedbuilder/ui/properties/BuildBehaviourTab.java b/build/org.eclipse.cdt.managedbuilder.ui/src/org/eclipse/cdt/managedbuilder/ui/properties/BuildBehaviourTab.java index 0193eeb2362..543259ae733 100644 --- a/build/org.eclipse.cdt.managedbuilder.ui/src/org/eclipse/cdt/managedbuilder/ui/properties/BuildBehaviourTab.java +++ b/build/org.eclipse.cdt.managedbuilder.ui/src/org/eclipse/cdt/managedbuilder/ui/properties/BuildBehaviourTab.java @@ -25,7 +25,6 @@ import org.eclipse.cdt.managedbuilder.internal.core.Configuration; import org.eclipse.cdt.managedbuilder.internal.core.MultiConfiguration; import org.eclipse.cdt.managedbuilder.internal.ui.Messages; import org.eclipse.cdt.newmake.core.IMakeBuilderInfo; -import org.eclipse.cdt.newmake.core.IMakeCommonBuildInfo; import org.eclipse.cdt.ui.newui.AbstractCPropertyTab; import org.eclipse.cdt.ui.newui.ICPropertyProvider; import org.eclipse.core.runtime.CoreException; @@ -231,12 +230,12 @@ public class BuildBehaviourTab extends AbstractCBuildPropertyTab { * 0: isStopOnError * 1: supportsStopOnError(true) * 2: bld.supportsStopOnError(false) - * 3: cfg.getParallelDef() + * 3: N/A * Mode 2: * 0: b.isAutoBuildEnable() * 1: b.isIncrementalBuildEnabled() * 2: b.isCleanBuildEnabled() - * 3: getParallelDef() + * 3: N/A */ static int[] calc3states(ICPropertyProvider p, IConfiguration mcfg, int mode) { if (p.isMultiCfg() && mcfg instanceof ICMultiItemsHolder) { @@ -253,8 +252,7 @@ public class BuildBehaviourTab extends AbstractCBuildPropertyTab { (m1 ? bldr0.supportsStopOnError(true) : bldr0.isIncrementalBuildEnabled() ); b[2] = m0 ? bldr0.canKeepEnvironmentVariablesInBuildfile() : (m1 ? bldr0.supportsStopOnError(false) : bldr0.isCleanBuildEnabled()); - b[3] = m0 ? bldr0.keepEnvironmentVariablesInBuildfile() : - (m1 ? ((Configuration)cfgs[0]).getParallelDef() : getParallelDef(mcfg)); + b[3] = m0 ? bldr0.keepEnvironmentVariablesInBuildfile() : false; for (int i=1; i