From 2b37dffcf30c985485ae323accc1c238ffc5ef5d Mon Sep 17 00:00:00 2001 From: Philipp Wollermann Date: Tue, 9 Feb 2016 12:42:56 +0000 Subject: Make The Build Faster: Use AutoCloseable when dealing with resources. -- MOS_MIGRATED_REVID=114204404 --- .../build/lib/actions/ResourceManager.java | 32 ++++++++++++++--- .../devtools/build/lib/exec/SymlinkTreeHelper.java | 7 ++-- .../lib/rules/test/StandaloneTestStrategy.java | 40 +++++++++------------- .../build/lib/skyframe/SkyframeActionExecutor.java | 8 +++-- 4 files changed, 53 insertions(+), 34 deletions(-) (limited to 'src/main/java') diff --git a/src/main/java/com/google/devtools/build/lib/actions/ResourceManager.java b/src/main/java/com/google/devtools/build/lib/actions/ResourceManager.java index e87e87e697..c263b2d05f 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ResourceManager.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ResourceManager.java @@ -60,6 +60,31 @@ import java.util.concurrent.CountDownLatch; @ThreadSafe public class ResourceManager { + /** + * A handle returned by {@link #acquireResources(ActionMetadata, ResourceSet)} that must be closed + * in order to free the resources again. + */ + public static class ResourceHandle implements AutoCloseable { + final ResourceManager rm; + final ActionMetadata actionMetadata; + final ResourceSet resourceSet; + + public ResourceHandle( + ResourceManager rm, ActionMetadata actionMetadata, ResourceSet resources) { + this.rm = rm; + this.actionMetadata = actionMetadata; + this.resourceSet = resources; + } + + /** + * Closing the ResourceHandle releases the resources associated with it. + */ + @Override + public void close() { + rm.releaseResources(actionMetadata, resourceSet); + } + } + private EventBus eventBus; private final ThreadLocal threadLocked = new ThreadLocal() { @@ -135,8 +160,7 @@ public class ResourceManager { /** * Resets resource manager state and releases all thread locks. - * Note - it does not reset available resources. Use - * separate call to setAvailableResoures(). + * Note - it does not reset available resources. Use separate call to setAvailableResources(). */ public synchronized void resetResourceUsage() { usedCpu = 0; @@ -167,7 +191,6 @@ public class ResourceManager { /** * Specify how much of the available RAM we should allow to be used. - * This has no effect if autosensing is enabled. */ public synchronized void setRamUtilizationPercentage(int percentage) { ramUtilizationPercentage = percentage; @@ -177,7 +200,7 @@ public class ResourceManager { * Acquires requested resource set. Will block if resource is not available. * NB! This method must be thread-safe! */ - public void acquireResources(ActionMetadata owner, ResourceSet resources) + public ResourceHandle acquireResources(ActionMetadata owner, ResourceSet resources) throws InterruptedException { Preconditions.checkNotNull(resources); AutoProfiler p = profiled(owner, ProfilerTask.ACTION_LOCK); @@ -198,6 +221,7 @@ public class ResourceManager { p.complete(); } } + return new ResourceHandle(this, owner, resources); } /** diff --git a/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java b/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java index 9534f5c591..ed87aea887 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java +++ b/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java @@ -21,6 +21,7 @@ import com.google.devtools.build.lib.actions.ActionExecutionContext; import com.google.devtools.build.lib.actions.BaseSpawn; import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.ResourceManager; +import com.google.devtools.build.lib.actions.ResourceManager.ResourceHandle; import com.google.devtools.build.lib.actions.ResourceSet; import com.google.devtools.build.lib.analysis.config.BinTools; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; @@ -110,13 +111,11 @@ public final class SymlinkTreeHelper { BinTools binTools) throws ExecException, InterruptedException { List args = getSpawnArgumentList( actionExecutionContext.getExecutor().getExecRoot(), binTools); - try { - ResourceManager.instance().acquireResources(action, RESOURCE_SET); + try (ResourceHandle handle = + ResourceManager.instance().acquireResources(action, RESOURCE_SET)) { actionExecutionContext.getExecutor().getSpawnActionContext(action.getMnemonic()).exec( new BaseSpawn.Local(args, ImmutableMap.of(), action), actionExecutionContext); - } finally { - ResourceManager.instance().releaseResources(action, RESOURCE_SET); } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/StandaloneTestStrategy.java b/src/main/java/com/google/devtools/build/lib/rules/test/StandaloneTestStrategy.java index 1cddbf6791..11b77f564c 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/test/StandaloneTestStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/rules/test/StandaloneTestStrategy.java @@ -23,6 +23,7 @@ import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.ExecutionStrategy; import com.google.devtools.build.lib.actions.Executor; import com.google.devtools.build.lib.actions.ResourceManager; +import com.google.devtools.build.lib.actions.ResourceManager.ResourceHandle; import com.google.devtools.build.lib.actions.ResourceSet; import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.actions.TestExecException; @@ -116,34 +117,27 @@ public class StandaloneTestStrategy extends TestStrategy { throw new EnvironmentalExecException("Could not create TEST_TMPDIR " + testTmpDir, e); } - ResourceSet resources = null; - FileOutErr fileOutErr = null; try { FileSystemUtils.createDirectoryAndParents(workingDirectory); - fileOutErr = new FileOutErr(action.getTestLog().getPath(), - action.resolve(actionExecutionContext.getExecutor().getExecRoot()).getTestStderr()); - - resources = action.getTestProperties() - .getLocalResourceUsage(executionOptions.usingLocalTestJobs()); - ResourceManager.instance().acquireResources(action, resources); - TestResultData data = execute( - actionExecutionContext.withFileOutErr(fileOutErr), spawn, action); - appendStderr(fileOutErr.getOutputFile(), fileOutErr.getErrorFile()); - finalizeTest(actionExecutionContext, action, data); + + ResourceSet resources = + action.getTestProperties().getLocalResourceUsage(executionOptions.usingLocalTestJobs()); + + try (FileOutErr fileOutErr = + new FileOutErr( + action.getTestLog().getPath(), + action + .resolve(actionExecutionContext.getExecutor().getExecRoot()) + .getTestStderr()); + ResourceHandle handle = ResourceManager.instance().acquireResources(action, resources)) { + TestResultData data = + execute(actionExecutionContext.withFileOutErr(fileOutErr), spawn, action); + appendStderr(fileOutErr.getOutputFile(), fileOutErr.getErrorFile()); + finalizeTest(actionExecutionContext, action, data); + } } catch (IOException e) { executor.getEventHandler().handle(Event.error("Caught I/O exception: " + e)); throw new EnvironmentalExecException("unexpected I/O exception", e); - } finally { - if (resources != null) { - ResourceManager.instance().releaseResources(action, resources); - } - try { - if (fileOutErr != null) { - fileOutErr.close(); - } - } catch (IOException e) { - // If the close fails, there is little we can do. - } } } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java index 27310f0a4a..39adae3b33 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java @@ -51,6 +51,7 @@ import com.google.devtools.build.lib.actions.NotifyOnActionCacheHit; import com.google.devtools.build.lib.actions.PackageRootResolutionException; import com.google.devtools.build.lib.actions.PackageRootResolver; import com.google.devtools.build.lib.actions.ResourceManager; +import com.google.devtools.build.lib.actions.ResourceManager.ResourceHandle; import com.google.devtools.build.lib.actions.ResourceSet; import com.google.devtools.build.lib.actions.TargetOutOfDateException; import com.google.devtools.build.lib.actions.cache.MetadataHandler; @@ -706,20 +707,21 @@ public final class SkyframeActionExecutor implements ActionExecutionContextFacto postEvent(new ActionStartedEvent(action, actionStartTime)); ResourceSet estimate = action.estimateResourceConsumption(executorEngine); ActionExecutionStatusReporter statusReporter = statusReporterRef.get(); + ResourceHandle handle = null; try { if (estimate == null || estimate == ResourceSet.ZERO) { statusReporter.setRunningFromBuildData(action); } else { // If estimated resource consumption is null, action will manually call // resource manager when it knows what resources are needed. - resourceManager.acquireResources(action, estimate); + handle = resourceManager.acquireResources(action, estimate); } boolean outputDumped = executeActionTask(action, context); completeAction(action, context.getMetadataHandler(), context.getFileOutErr(), outputDumped); } finally { - if (estimate != null) { - resourceManager.releaseResources(action, estimate); + if (handle != null) { + handle.close(); } statusReporter.remove(action); postEvent(new ActionCompletionEvent(actionStartTime, action)); -- cgit v1.2.3