From d643770a7c6a0ce2302b7d8605792946f0fe72f3 Mon Sep 17 00:00:00 2001 From: Alex Osborne Date: Tue, 22 Jun 2021 16:03:29 +0900 Subject: [PATCH 1/3] Put slowest unit tests behind a -DrunSlowTests=true option Shaves about 2 minutes off the Heritrix build time. --- .../org/archive/util/BloomFilterTest.java | 22 ++++++-- .../ObjectIdentityBdbManualCacheTest.java | 50 +++++++++++-------- 2 files changed, 47 insertions(+), 25 deletions(-) diff --git a/commons/src/test/java/org/archive/util/BloomFilterTest.java b/commons/src/test/java/org/archive/util/BloomFilterTest.java index 651fef94..ea74019c 100644 --- a/commons/src/test/java/org/archive/util/BloomFilterTest.java +++ b/commons/src/test/java/org/archive/util/BloomFilterTest.java @@ -21,7 +21,11 @@ package org.archive.util; import java.util.Random; -import junit.framework.TestCase; +import org.junit.Ignore; +import org.junit.Test; + +import static org.junit.Assert.*; +import static org.junit.Assume.assumeTrue; /** @@ -30,7 +34,7 @@ import junit.framework.TestCase; * @author gojomo * @version $Date: 2009-11-19 14:39:53 -0800 (Thu, 19 Nov 2009) $, $Revision: 6674 $ */ -public abstract class BloomFilterTest extends TestCase { +public abstract class BloomFilterTest { abstract BloomFilter createBloom(long n, int d, Random random); @@ -70,6 +74,8 @@ public abstract class BloomFilterTest extends TestCase { * Renamed to non-'test' name so not automatically run, because can * take 15+ minutes to complete. */ + @Test + @Ignore public void xestOversized() { trialWithParameters(200000000,22,200000000,32000000); } @@ -81,15 +87,23 @@ public abstract class BloomFilterTest extends TestCase { * Renamed to non-'test' name so not automatically run, because can * take 15+ minutes to complete. */ + @Test + @Ignore public void xestDefaultFull() { trialWithParameters(125000000,22,125000000,34000000); } - + + @Test public void testDefaultAbbreviated() { + assumeTrue("use -DrunSlowTests=true to enable this test (it takes about 25 seconds)", + "true".equals(System.getProperty("runSlowTests"))); trialWithParameters(125000000,22,17000000,0); } - + + @Test public void testSmall() { + assumeTrue("use -DrunSlowTests=true to enable this test (it takes about 20 seconds)", + "true".equals(System.getProperty("runSlowTests"))); trialWithParameters(10000000, 20, 10000000, 10000000); } diff --git a/commons/src/test/java/org/archive/util/ObjectIdentityBdbManualCacheTest.java b/commons/src/test/java/org/archive/util/ObjectIdentityBdbManualCacheTest.java index 2873be5c..5685d3d0 100644 --- a/commons/src/test/java/org/archive/util/ObjectIdentityBdbManualCacheTest.java +++ b/commons/src/test/java/org/archive/util/ObjectIdentityBdbManualCacheTest.java @@ -18,25 +18,32 @@ */ package org.archive.util; -import java.io.File; -import java.util.HashMap; -import java.util.concurrent.atomic.AtomicInteger; - -import org.apache.commons.io.FileUtils; -import org.archive.util.bdbje.EnhancedEnvironment; +import java.io.File; +import java.util.HashMap; +import java.util.concurrent.atomic.AtomicInteger; + +import org.apache.commons.io.FileUtils; +import org.archive.util.bdbje.EnhancedEnvironment; +import org.junit.*; +import org.junit.rules.TemporaryFolder; + +import static org.junit.Assert.*; +import static org.junit.Assume.assumeTrue; /** * @author stack * @author gojomo * @version $Date: 2009-08-03 23:50:43 -0700 (Mon, 03 Aug 2009) $, $Revision: 6434 $ */ -public class ObjectIdentityBdbManualCacheTest extends TmpDirTestCase { +public class ObjectIdentityBdbManualCacheTest { + @Rule + public TemporaryFolder tmpFolder = new TemporaryFolder(); EnhancedEnvironment env; private ObjectIdentityBdbManualCache>> cache; - - protected void setUp() throws Exception { - super.setUp(); - File envDir = new File(getTmpDir(),"ObjectIdentityBdbCacheTest"); + + @Before + public void setUp() throws Exception { + File envDir = tmpFolder.newFolder("ObjectIdentityBdbCacheTest"); org.archive.util.FileUtils.ensureWriteableDirectory(envDir); FileUtils.deleteDirectory(envDir); org.archive.util.FileUtils.ensureWriteableDirectory(envDir); @@ -44,16 +51,19 @@ public class ObjectIdentityBdbManualCacheTest extends TmpDirTestCase { this.cache = new ObjectIdentityBdbManualCache>>(); this.cache.initialize(env,"setUpCache",IdentityCacheableWrapper.class, env.getClassCatalog()); } - - protected void tearDown() throws Exception { + + @After + public void tearDown() throws Exception { this.cache.close(); File envDir = env.getHome(); env.close(); FileUtils.deleteDirectory(envDir); - super.tearDown(); } - + + @Test public void testReadConsistencyUnderLoad() throws Exception { + assumeTrue("use -DrunSlowTests=true to enable this test (it takes about 1 minute)", + "true".equals(System.getProperty("runSlowTests"))); final ObjectIdentityBdbManualCache> cbdbmap = new ObjectIdentityBdbManualCache<>(); cbdbmap.initialize(env, @@ -95,7 +105,8 @@ public class ObjectIdentityBdbManualCacheTest extends TmpDirTestCase { } // SUCCESS } - + + @Test public void testBackingDbGetsUpdated() { // Set up values. final String value = "value"; @@ -129,6 +140,8 @@ public class ObjectIdentityBdbManualCacheTest extends TmpDirTestCase { * expunged of otherwise unreferenced entries as expected. * @throws InterruptedException */ + @Test + @Ignore public void xestMemMapCleared() throws InterruptedException { TestUtils.forceScarceMemory(); System.gc(); // minimize effects of earlier test heap use @@ -160,9 +173,4 @@ public class ObjectIdentityBdbManualCacheTest extends TmpDirTestCase { System.out.println(cache.size()+","+cache.memMap.size()+","+cache.memMap.keySet().size()+","+cache.memMap.values().size()+","+countNonNull); assertEquals("memMap not cleared", 0, cache.memMap.size()); } - - - public static void main(String [] args) { - junit.textui.TestRunner.run(ObjectIdentityBdbManualCacheTest.class); - } } From 16a49d35275bdf0128e7933f3fc12b6e63929496 Mon Sep 17 00:00:00 2001 From: Alex Osborne Date: Tue, 22 Jun 2021 13:40:09 +0900 Subject: [PATCH 2/3] Remove arbitrary 1.5 second sleep() when launching jobs I think the sleep is supposed to make launch() not return until the job has actually been launched but it doesn't work as launch() and getCrawlController() are both synchronized therefore the launcher thread can't actually call startContext() until launch() returns after sleeping. So let's replace the sleep call with join and unsynchronize launch() so it doesn't deadlock. All the relevant methods it calls seem to be synchronized so I think it's no worse to not synchronize it itself. --- .../main/java/org/archive/crawler/framework/CrawlJob.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/engine/src/main/java/org/archive/crawler/framework/CrawlJob.java b/engine/src/main/java/org/archive/crawler/framework/CrawlJob.java index 02bdbe2e..c96aa000 100644 --- a/engine/src/main/java/org/archive/crawler/framework/CrawlJob.java +++ b/engine/src/main/java/org/archive/crawler/framework/CrawlJob.java @@ -408,7 +408,7 @@ public class CrawlJob implements Comparable, ApplicationListener, ApplicationListener Date: Tue, 22 Jun 2021 15:39:40 +0900 Subject: [PATCH 3/3] Speed up the unit tests by changing some 1s polling sleeps to 250ms Lots of 1 second sleeps add up fast. On my PC this reduces the runtime of `mvn clean test` from about 3m 40s to about 1m 50s. Ideally we'd probably re-architect some this to use a notification mechanism instead of polling but that's easier said than done and this is a pretty big improvement by itself. --- .../src/main/java/org/archive/crawler/framework/Engine.java | 6 +++--- .../java/org/archive/crawler/frontier/AbstractFrontier.java | 6 +++--- .../org/archive/crawler/frontier/WorkQueueFrontier.java | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/engine/src/main/java/org/archive/crawler/framework/Engine.java b/engine/src/main/java/org/archive/crawler/framework/Engine.java index 31a018d3..875c3e6b 100644 --- a/engine/src/main/java/org/archive/crawler/framework/Engine.java +++ b/engine/src/main/java/org/archive/crawler/framework/Engine.java @@ -317,7 +317,7 @@ public class Engine { return true; } try { - Thread.sleep(500); + Thread.sleep(250); } catch (InterruptedException e) { break; } @@ -329,8 +329,8 @@ public class Engine { break; } try { - // wait an extra second for good measure - Thread.sleep(1000); + // wait an extra quarter second for good measure + Thread.sleep(250); } catch (InterruptedException e) { // ignore } diff --git a/engine/src/main/java/org/archive/crawler/frontier/AbstractFrontier.java b/engine/src/main/java/org/archive/crawler/frontier/AbstractFrontier.java index 4729f7b6..dd0df695 100644 --- a/engine/src/main/java/org/archive/crawler/frontier/AbstractFrontier.java +++ b/engine/src/main/java/org/archive/crawler/frontier/AbstractFrontier.java @@ -364,7 +364,7 @@ public abstract class AbstractFrontier } reachedState(reachedState); - Thread.sleep(1000); + Thread.sleep(250); if(isEmpty()&&targetState==State.RUN) { requestState(State.EMPTY); @@ -384,7 +384,7 @@ public abstract class AbstractFrontier reachedState(State.PAUSE); } - Thread.sleep(1000); + Thread.sleep(250); } break; case FINISH: @@ -393,7 +393,7 @@ public abstract class AbstractFrontier outboundLock.writeLock().lock(); // process all inbound while (getInProcessCount()>0) { - Thread.sleep(1000); + Thread.sleep(250); } logger.fine("0 urls in process, running final tasks"); finalTasks(); diff --git a/engine/src/main/java/org/archive/crawler/frontier/WorkQueueFrontier.java b/engine/src/main/java/org/archive/crawler/frontier/WorkQueueFrontier.java index 7d7a3b00..b37a75b2 100644 --- a/engine/src/main/java/org/archive/crawler/frontier/WorkQueueFrontier.java +++ b/engine/src/main/java/org/archive/crawler/frontier/WorkQueueFrontier.java @@ -727,7 +727,7 @@ implements Closeable, // next time if(getTotalEligibleInactiveQueues()==0) { try { - Thread.sleep(1000); + Thread.sleep(250); } catch (InterruptedException e) { // }