Index: trunk/src/org/openstreetmap/josm/data/cache/HostLimitQueue.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/cache/HostLimitQueue.java (revision 14260) +++ trunk/src/org/openstreetmap/josm/data/cache/HostLimitQueue.java (revision 14261) @@ -9,5 +9,4 @@ import java.util.concurrent.LinkedBlockingDeque; import java.util.concurrent.Semaphore; -import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; @@ -34,10 +33,4 @@ private final int hostLimit; - private ThreadPoolExecutor executor; - - private int corePoolSize; - - private int maximumPoolSize; - /** * Creates an unbounded queue @@ -46,4 +39,14 @@ public HostLimitQueue(int hostLimit) { super(); // create unbounded queue + this.hostLimit = hostLimit; + } + + /** + * Creates bounded queue + * @param hostLimit how many parallel calls to host to allow + * @param queueLimit how deep the queue should be + */ + public HostLimitQueue(int hostLimit, int queueLimit) { + super(queueLimit); // create bounded queue this.hostLimit = hostLimit; } @@ -117,35 +120,4 @@ } - /** - * Set the executor for which this queue works. It's needed to spawn new threads. - * See: http://stackoverflow.com/questions/9622599/java-threadpoolexecutor-strategy-direct-handoff-with-queue# - * - * @param executor executor for which this queue works - */ - public void setExecutor(ThreadPoolExecutor executor) { - this.executor = executor; - this.maximumPoolSize = executor.getMaximumPoolSize(); - this.corePoolSize = executor.getCorePoolSize(); - } - - @Override - public boolean offer(Runnable e) { - if (!super.offer(e)) { - return false; - } - - if (executor != null) { - // See: http://stackoverflow.com/questions/9622599/java-threadpoolexecutor-strategy-direct-handoff-with-queue# - // force spawn of a thread if not reached maximum - int currentPoolSize = executor.getPoolSize(); - if (currentPoolSize < maximumPoolSize - && currentPoolSize >= corePoolSize) { - executor.setCorePoolSize(currentPoolSize + 1); - executor.setCorePoolSize(corePoolSize); - } - } - return true; - } - private Semaphore getSemaphore(JCSCachedTileLoaderJob job) { String host; Index: trunk/src/org/openstreetmap/josm/data/imagery/TMSCachedTileLoader.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/imagery/TMSCachedTileLoader.java (revision 14260) +++ trunk/src/org/openstreetmap/josm/data/imagery/TMSCachedTileLoader.java (revision 14261) @@ -68,14 +68,22 @@ */ public static ThreadPoolExecutor getNewThreadPoolExecutor(String nameFormat, int workers) { - HostLimitQueue workQueue = new HostLimitQueue(HOST_LIMIT.get().intValue()); + return getNewThreadPoolExecutor(nameFormat, workers, HOST_LIMIT.get().intValue()); + } + + /** + * @param nameFormat see {@link Utils#newThreadFactory(String, int)} + * @param workers number of worker thread to keep + * @param hostLimit number of concurrent downloads per host allowed + * @return new ThreadPoolExecutor that will use a @see HostLimitQueue based queue + */ + public static ThreadPoolExecutor getNewThreadPoolExecutor(String nameFormat, int workers, int hostLimit) { ThreadPoolExecutor executor = new ThreadPoolExecutor( - 0, // 0 so for unused thread pools threads will eventually die, freeing also the threadpool - workers, // do not this number of threads - 300, // keepalive for thread + workers, // keep core pool the same size as max, as we use unbounded queue so there will + workers, // be never more threads than corePoolSize + 300, // keep alive for thread TimeUnit.SECONDS, - workQueue, + new HostLimitQueue(hostLimit), Utils.newThreadFactory(nameFormat, Thread.NORM_PRIORITY) ); - workQueue.setExecutor(executor); return executor; } Index: trunk/src/org/openstreetmap/josm/data/imagery/WMSCachedTileLoader.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/imagery/WMSCachedTileLoader.java (revision 14260) +++ trunk/src/org/openstreetmap/josm/data/imagery/WMSCachedTileLoader.java (revision 14261) @@ -24,5 +24,5 @@ /** - * Creates a TileLoader with separate WMS downloader. + * Creates a TileLoader with separate WMS download thread pool. * * @param listener that will be notified when tile is loaded Index: trunk/test/unit/org/openstreetmap/josm/data/cache/HostLimitQueueTest.java =================================================================== --- trunk/test/unit/org/openstreetmap/josm/data/cache/HostLimitQueueTest.java (revision 14260) +++ trunk/test/unit/org/openstreetmap/josm/data/cache/HostLimitQueueTest.java (revision 14261) @@ -14,8 +14,8 @@ import org.junit.Rule; import org.junit.Test; +import org.openstreetmap.josm.data.imagery.TMSCachedTileLoader; import org.openstreetmap.josm.data.imagery.TileJobOptions; import org.openstreetmap.josm.testutils.JOSMTestRules; import org.openstreetmap.josm.tools.Logging; -import org.openstreetmap.josm.tools.Utils; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; @@ -33,17 +33,4 @@ public JOSMTestRules test = new JOSMTestRules().preferences().timeout(20 * 1000); - private static ThreadPoolExecutor getNewThreadPoolExecutor(String nameFormat, int workers, int queueLimit) { - HostLimitQueue workQueue = new HostLimitQueue(queueLimit); - ThreadPoolExecutor executor = new ThreadPoolExecutor( - 0, // 0 so for unused thread pools threads will eventually die, freeing also the threadpool - workers, // do not this number of threads - 300, // keepalive for thread - TimeUnit.SECONDS, - workQueue, - Utils.newThreadFactory(nameFormat, Thread.NORM_PRIORITY) - ); - workQueue.setExecutor(executor); - return executor; - } /** @@ -94,5 +81,5 @@ @Test public void testSingleThreadPerHost() throws Exception { - ThreadPoolExecutor tpe = getNewThreadPoolExecutor("test-%d", 3, 1); + ThreadPoolExecutor tpe = TMSCachedTileLoader.getNewThreadPoolExecutor("test-%d", 3, 1); ICacheAccess cache = JCSCacheManager.getCache("test", 3, 0, ""); AtomicInteger counter = new AtomicInteger(0); @@ -107,8 +94,9 @@ assertEquals(10, counter.get()); // although there are 3 threads, we can make only 1 parallel call to localhost - // so it should take ~10 seconds to finish + // first three jobs will be not limited, as they spawn the thread + // so it should take ~8 seconds to finish // if it's shorter, it means that host limit does not work - assertTrue("Expected duration between 9 and 11 seconds not met. Actual duration: " + (duration /1000), - duration < 11*1000 & duration > 9*1000); + assertTrue("Expected duration between 8 and 11 seconds not met. Actual duration: " + (duration /1000), + duration < 11*1000 & duration > 8*1000); } @@ -119,5 +107,5 @@ @Test public void testMultipleThreadPerHost() throws Exception { - ThreadPoolExecutor tpe = getNewThreadPoolExecutor("test-%d", 3, 2); + ThreadPoolExecutor tpe = TMSCachedTileLoader.getNewThreadPoolExecutor("test-%d", 3, 2); ICacheAccess cache = JCSCacheManager.getCache("test", 3, 0, ""); AtomicInteger counter = new AtomicInteger(0); @@ -144,5 +132,5 @@ @Test public void testTwoHosts() throws Exception { - ThreadPoolExecutor tpe = getNewThreadPoolExecutor("test-%d", 3, 1); + ThreadPoolExecutor tpe = TMSCachedTileLoader.getNewThreadPoolExecutor("test-%d", 3, 1); ICacheAccess cache = JCSCacheManager.getCache("test", 3, 0, ""); AtomicInteger counter = new AtomicInteger(0);