From 946812db1b839e893f888794077114bb62cb6844 Mon Sep 17 00:00:00 2001 From: Philipp Wollermann Date: Mon, 29 Aug 2016 08:35:33 +0000 Subject: 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 --- .../build/lib/actions/ResourceManagerTest.java | 126 ++++++++++++++++++--- 1 file changed, 112 insertions(+), 14 deletions(-) (limited to 'src/test/java/com/google') diff --git a/src/test/java/com/google/devtools/build/lib/actions/ResourceManagerTest.java b/src/test/java/com/google/devtools/build/lib/actions/ResourceManagerTest.java index 2c4177994f..d05123e70c 100644 --- a/src/test/java/com/google/devtools/build/lib/actions/ResourceManagerTest.java +++ b/src/test/java/com/google/devtools/build/lib/actions/ResourceManagerTest.java @@ -73,7 +73,7 @@ public class ResourceManagerTest { rm.releaseResources(resourceOwner, ResourceSet.create(ram, cpu, io, tests)); } - private void validate (int count) { + private void validate(int count) { assertEquals(count, counter.incrementAndGet()); } @@ -124,7 +124,15 @@ public class ResourceManagerTest { // When a request for CPU is made that would slightly overallocate CPU, // Then the request succeeds: - assertThat(acquireNonblocking(0, 0.6, 0, 0)).isNotNull(); + TestThread thread1 = + new TestThread() { + @Override + public void runTest() throws Exception { + assertThat(acquireNonblocking(0, 0.6, 0, 0)).isNotNull(); + } + }; + thread1.start(); + thread1.joinAndAssertState(10000); } @Test @@ -136,20 +144,36 @@ public class ResourceManagerTest { // When a request for a large CPU allocation is made, // Then the request succeeds: - assertThat(acquireNonblocking(0, 0.99, 0, 0)).isNotNull(); + TestThread thread1 = + new TestThread() { + @Override + public void runTest() throws Exception { + assertThat(acquireNonblocking(0, 0.99, 0, 0)).isNotNull(); + // Cleanup + release(0, 0.99, 0, 0); + } + }; + thread1.start(); + thread1.joinAndAssertState(10000); // Cleanup - release(0, 1.089, 0, 0); + release(0, 0.099, 0, 0); assertFalse(rm.inUse()); - // Given that CPU has a large initial allocation: acquire(0, 0.99, 0, 0); // When a request for a small CPU allocation is made, // Then the request fails: - assertThat(acquireNonblocking(0, 0.099, 0, 0)).isNull(); - + TestThread thread2 = + new TestThread() { + @Override + public void runTest() throws Exception { + assertThat(acquireNonblocking(0, 0.099, 0, 0)).isNull(); + } + }; + thread2.start(); + thread2.joinAndAssertState(10000); // Note that this behavior is surprising and probably not intended. } @@ -162,7 +186,15 @@ public class ResourceManagerTest { // When a request for RAM is made that would slightly overallocate RAM, // Then the request fails: - assertThat(acquireNonblocking(600, 0, 0, 0)).isNull(); + TestThread thread1 = + new TestThread() { + @Override + public void runTest() throws Exception { + assertThat(acquireNonblocking(600, 0, 0, 0)).isNull(); + } + }; + thread1.start(); + thread1.joinAndAssertState(10000); } @Test @@ -174,7 +206,15 @@ public class ResourceManagerTest { // When a request for IO is made that would slightly overallocate IO, // Then the request fails: - assertThat(acquireNonblocking(0, 0, 0.6, 0)).isNull(); + TestThread thread1 = + new TestThread() { + @Override + public void runTest() throws Exception { + assertThat(acquireNonblocking(0, 0, 0.6, 0)).isNull(); + } + }; + thread1.start(); + thread1.joinAndAssertState(10000); } @Test @@ -186,7 +226,15 @@ public class ResourceManagerTest { // When a request for tests is made that would slightly overallocate tests, // Then the request fails: - assertThat(acquireNonblocking(0, 0, 0, 2)).isNull(); + TestThread thread1 = + new TestThread() { + @Override + public void runTest() throws Exception { + assertThat(acquireNonblocking(0, 0, 0, 2)).isNull(); + } + }; + thread1.start(); + thread1.joinAndAssertState(10000); } @Test @@ -323,7 +371,11 @@ public class ResourceManagerTest { @Test public void testOutOfOrderAllocation() throws Exception { + final CyclicBarrier sync3 = new CyclicBarrier(2); + final CyclicBarrier sync4 = new CyclicBarrier(2); + assertFalse(rm.inUse()); + TestThread thread1 = new TestThread () { @Override public void runTest() throws Exception { sync.await(); @@ -333,6 +385,7 @@ public class ResourceManagerTest { sync.await(); } }; + TestThread thread2 = new TestThread() { @Override public void runTest() throws Exception { // Wait till other thread will be locked @@ -349,24 +402,69 @@ public class ResourceManagerTest { release(200, 0.5, 0, 0); } }; - acquire(900, 0.9, 0, 0); + + TestThread thread3 = + new TestThread() { + @Override + public void runTest() throws Exception { + acquire(100, 0.4, 0, 0); + sync3.await(); + sync3.await(); + release(100, 0.4, 0, 0); + } + }; + + TestThread thread4 = + new TestThread() { + @Override + public void runTest() throws Exception { + acquire(750, 0.3, 0, 0); + sync4.await(); + sync4.await(); + release(750, 0.3, 0, 0); + } + }; + + // Lock 900 MB, 0.9 CPU in total (spread over three threads so that we can individually release + // parts of it). + acquire(50, 0.2, 0, 0); + thread3.start(); + thread4.start(); + sync3.await(1, TimeUnit.SECONDS); + sync4.await(1, TimeUnit.SECONDS); validate(1); + + // Start thread1, which will try to acquire 900 MB, 0.5 CPU, but can't, so it has to wait. thread1.start(); sync.await(1, TimeUnit.SECONDS); + + // Start thread2, which will successfully acquire and release 100 MB, 0.1 CPU. thread2.start(); + // Signal thread2 to acquire 200 MB and 0.5 CPU, which will block. sync2.await(1, TimeUnit.SECONDS); - //Waiting till both threads are locked. + + // Waiting till both threads are locked. while (rm.getWaitCount() < 2) { Thread.yield(); } + validate(3); // Thread1 is now first in the queue and Thread2 is second. - release(100, 0.4, 0, 0); // This allows Thread2 to continue out of order. + + // Release 100 MB, 0.4 CPU. This allows Thread2 to continue out of order. + sync3.await(1, TimeUnit.SECONDS); sync2.await(1, TimeUnit.SECONDS); - release(750, 0.3, 0, 0); // At this point thread1 will finally acquire resources. + + // Release 750 MB, 0.3 CPU. At this point thread1 will finally acquire resources. + sync4.await(1, TimeUnit.SECONDS); sync.await(1, TimeUnit.SECONDS); + + // Release all remaining resources. release(50, 0.2, 0, 0); thread1.join(); thread2.join(); + thread3.join(); + thread4.join(); + assertFalse(rm.inUse()); } -- cgit v1.2.3