diff options
author | Philipp Wollermann <philwo@google.com> | 2016-08-29 08:35:33 +0000 |
---|---|---|
committer | Klaus Aehlig <aehlig@google.com> | 2016-08-29 09:42:37 +0000 |
commit | 946812db1b839e893f888794077114bb62cb6844 (patch) | |
tree | 9c63e418a5ec2728651afb9bd5d7a5f0ef12cfe0 /src/main/java/com/google/devtools/build/lib | |
parent | 6c4dfd328ee00b02bcd9bd56e3e028af41b19023 (diff) |
Some little fixes to ResourceManager.
- Make sure that empty ResourceSets are always == ResourceSet.ZERO and use that for easier comparison.
- No longer allow nested resource acquisition, because it may lead to deadlocks.
--
MOS_MIGRATED_REVID=131567446
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib')
-rw-r--r-- | src/main/java/com/google/devtools/build/lib/actions/ResourceManager.java | 24 | ||||
-rw-r--r-- | src/main/java/com/google/devtools/build/lib/actions/ResourceSet.java | 7 |
2 files changed, 25 insertions, 6 deletions
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 c22a1c097f..9d5edc3153 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 @@ -201,7 +201,11 @@ public class ResourceManager { */ public ResourceHandle acquireResources(ActionExecutionMetadata owner, ResourceSet resources) throws InterruptedException { - Preconditions.checkNotNull(resources); + Preconditions.checkNotNull( + resources, "acquireResources called with resources == NULL during %s", owner); + Preconditions.checkState( + !threadHasResources(), "acquireResources with existing resource lock during %s", owner); + AutoProfiler p = profiled(owner, ProfilerTask.ACTION_LOCK); CountDownLatch latch = null; try { @@ -224,8 +228,7 @@ public class ResourceManager { } throw e; } finally { - threadLocked.set(resources.getCpuUsage() != 0 || resources.getMemoryMb() != 0 - || resources.getIoUsage() != 0 || resources.getLocalTestCount() != 0); + threadLocked.set(resources != ResourceSet.ZERO); acquired(owner); // Profile acquisition only if it waited for resource to become available. @@ -233,6 +236,7 @@ public class ResourceManager { p.complete(); } } + return new ResourceHandle(this, owner, resources); } @@ -242,7 +246,13 @@ public class ResourceManager { * @return a ResourceHandle iff the given resources were locked (all or nothing), null otherwise. */ public ResourceHandle tryAcquire(ActionExecutionMetadata owner, ResourceSet resources) { + Preconditions.checkNotNull( + resources, "tryAcquire called with resources == NULL during %s", owner); + Preconditions.checkState( + !threadHasResources(), "tryAcquire with existing resource lock during %s", owner); + boolean acquired = false; + synchronized (this) { if (areResourcesAvailable(resources)) { incrementResources(resources); @@ -251,8 +261,7 @@ public class ResourceManager { } if (acquired) { - threadLocked.set(resources.getCpuUsage() != 0 || resources.getMemoryMb() != 0 - || resources.getIoUsage() != 0 || resources.getLocalTestCount() != 0); + threadLocked.set(resources != ResourceSet.ZERO); acquired(owner); return new ResourceHandle(this, owner, resources); } @@ -313,6 +322,11 @@ public class ResourceManager { * <p>NB! This method must be thread-safe! */ public void releaseResources(ActionExecutionMetadata owner, ResourceSet resources) { + Preconditions.checkNotNull( + resources, "releaseResources called with resources == NULL during %s", owner); + Preconditions.checkState( + threadHasResources(), "releaseResources without resource lock during %s", owner); + boolean isConflict = false; AutoProfiler p = profiled(owner, ProfilerTask.ACTION_RELEASE); try { diff --git a/src/main/java/com/google/devtools/build/lib/actions/ResourceSet.java b/src/main/java/com/google/devtools/build/lib/actions/ResourceSet.java index d191e3cd53..9668eba2cb 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ResourceSet.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ResourceSet.java @@ -18,7 +18,6 @@ import com.google.common.base.Splitter; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.common.options.Converter; import com.google.devtools.common.options.OptionsParsingException; - import java.util.Iterator; import java.util.NoSuchElementException; @@ -63,6 +62,9 @@ public class ResourceSet { * local tests. */ public static ResourceSet createWithRamCpuIo(double memoryMb, double cpuUsage, double ioUsage) { + if (memoryMb == 0 && cpuUsage == 0 && ioUsage == 0) { + return ZERO; + } return new ResourceSet(memoryMb, cpuUsage, ioUsage, 0); } @@ -83,6 +85,9 @@ public class ResourceSet { */ public static ResourceSet create(double memoryMb, double cpuUsage, double ioUsage, int localTestCount) { + if (memoryMb == 0 && cpuUsage == 0 && ioUsage == 0 && localTestCount == 0) { + return ZERO; + } return new ResourceSet(memoryMb, cpuUsage, ioUsage, localTestCount); } |