From b305fe082ce61d3d5d28cc07b91f4bc478a551c3 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Fri, 15 Nov 2013 18:30:29 -0500 Subject: [PATCH] Added NamingThreadFactory to ensure that all code creating thread pool executors was marking the threads. Otherwise we get a bunch of pool-17-thread-5 with no clue who created so many threads or why. Also simplifies existing code which previously had to write one-off ThreadFactory implementations just to set a name. --- core/src/main/java/hudson/Proc.java | 3 +- core/src/main/java/hudson/model/Computer.java | 18 +----- .../main/java/hudson/model/UpdateCenter.java | 18 +----- .../main/java/hudson/triggers/SCMTrigger.java | 11 +++- .../main/java/hudson/util/CompressedFile.java | 2 +- .../java/hudson/util/NamingThreadFactory.java | 57 +++++++++++++++++++ .../main/java/jenkins/InitReactorRunner.java | 3 +- core/src/main/java/jenkins/model/Jenkins.java | 9 +-- core/src/main/java/jenkins/util/Timer.java | 17 +----- 9 files changed, 82 insertions(+), 56 deletions(-) create mode 100644 core/src/main/java/hudson/util/NamingThreadFactory.java diff --git a/core/src/main/java/hudson/Proc.java b/core/src/main/java/hudson/Proc.java index b80d814458..5114cbd2c4 100644 --- a/core/src/main/java/hudson/Proc.java +++ b/core/src/main/java/hudson/Proc.java @@ -28,6 +28,7 @@ import hudson.model.TaskListener; import hudson.remoting.Channel; import hudson.util.DaemonThreadFactory; import hudson.util.ExceptionCatchingThreadFactory; +import hudson.util.NamingThreadFactory; import hudson.util.NullStream; import hudson.util.StreamCopyThread; import hudson.util.ProcessTree; @@ -131,7 +132,7 @@ public abstract class Proc { */ public abstract OutputStream getStdin(); - private static final ExecutorService executor = Executors.newCachedThreadPool(new ExceptionCatchingThreadFactory(new DaemonThreadFactory())); + private static final ExecutorService executor = Executors.newCachedThreadPool(new ExceptionCatchingThreadFactory(new NamingThreadFactory(new DaemonThreadFactory(), "Proc.executor"))); /** * Like {@link #join} but can be given a maximum time to wait. diff --git a/core/src/main/java/hudson/model/Computer.java b/core/src/main/java/hudson/model/Computer.java index d409612394..219373d5ff 100644 --- a/core/src/main/java/hudson/model/Computer.java +++ b/core/src/main/java/hudson/model/Computer.java @@ -51,12 +51,14 @@ import hudson.slaves.RetentionStrategy; import hudson.slaves.WorkspaceList; import hudson.slaves.OfflineCause; import hudson.slaves.OfflineCause.ByCLI; +import hudson.util.DaemonThreadFactory; import hudson.util.EditDistance; import hudson.util.ExceptionCatchingThreadFactory; import hudson.util.RemotingDiagnostics; import hudson.util.RemotingDiagnostics.HeapDump; import hudson.util.RunList; import hudson.util.Futures; +import hudson.util.NamingThreadFactory; import jenkins.model.Jenkins; import org.kohsuke.args4j.Argument; import org.kohsuke.args4j.CmdLineException; @@ -85,8 +87,6 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; import java.util.concurrent.ExecutionException; -import java.util.concurrent.ThreadFactory; -import java.util.concurrent.atomic.AtomicInteger; import java.util.logging.LogRecord; import java.util.logging.Level; import java.util.logging.Logger; @@ -1060,19 +1060,7 @@ public /*transient*/ abstract class Computer extends Actionable implements Acces private static final long serialVersionUID = 1L; } - public static final ExecutorService threadPoolForRemoting = Executors.newCachedThreadPool(new ExceptionCatchingThreadFactory( - new ThreadFactory() { - - private final AtomicInteger threadNumber = new AtomicInteger(1); - - @Override - public Thread newThread(Runnable r) { - Thread t = new Thread(r); - t.setName("Jenkins-Remoting-Thread-"+threadNumber.getAndIncrement()); - t.setDaemon(true); - return t; - } - })); + public static final ExecutorService threadPoolForRemoting = Executors.newCachedThreadPool(new ExceptionCatchingThreadFactory(new NamingThreadFactory(new DaemonThreadFactory(), "Computer.threadPoolForRemoting"))); // // diff --git a/core/src/main/java/hudson/model/UpdateCenter.java b/core/src/main/java/hudson/model/UpdateCenter.java index 068c480171..344ab98582 100644 --- a/core/src/main/java/hudson/model/UpdateCenter.java +++ b/core/src/main/java/hudson/model/UpdateCenter.java @@ -44,6 +44,7 @@ import hudson.security.ACL; import hudson.util.DaemonThreadFactory; import hudson.util.FormValidation; import hudson.util.HttpResponses; +import hudson.util.NamingThreadFactory; import hudson.util.PersistedList; import hudson.util.XStream2; import jenkins.RestartRequiredException; @@ -81,7 +82,6 @@ import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; -import java.util.concurrent.ThreadFactory; import java.util.concurrent.atomic.AtomicInteger; import java.util.jar.Attributes; import java.util.jar.JarFile; @@ -130,25 +130,13 @@ public class UpdateCenter extends AbstractModelObject implements Saveable, OnMas * @since 1.501 */ private final ExecutorService installerService = new AtmostOneThreadExecutor( - new DaemonThreadFactory(new ThreadFactory() { - public Thread newThread(Runnable r) { - Thread t = new Thread(r); - t.setName("Update center installer thread"); - return t; - } - })); + new NamingThreadFactory(new DaemonThreadFactory(), "Update center installer thread")); /** * An {@link ExecutorService} for updating UpdateSites. */ protected final ExecutorService updateService = Executors.newCachedThreadPool( - new DaemonThreadFactory(new ThreadFactory() { - public Thread newThread(Runnable r) { - Thread t = new Thread(r); - t.setName("Update site data downloader"); - return t; - } - })); + new NamingThreadFactory(new DaemonThreadFactory(), "Update site data downloader")); /** * List of created {@link UpdateCenterJob}s. Access needs to be synchronized. diff --git a/core/src/main/java/hudson/triggers/SCMTrigger.java b/core/src/main/java/hudson/triggers/SCMTrigger.java index daad3527bd..0bcea37220 100644 --- a/core/src/main/java/hudson/triggers/SCMTrigger.java +++ b/core/src/main/java/hudson/triggers/SCMTrigger.java @@ -39,6 +39,7 @@ import hudson.model.AdministrativeMonitor; import hudson.model.Run; import hudson.util.FlushProofOutputStream; import hudson.util.FormValidation; +import hudson.util.NamingThreadFactory; import hudson.util.StreamTaskListener; import hudson.util.TimeUnit2; import hudson.util.SequentialExecutionQueue; @@ -61,6 +62,7 @@ import java.util.concurrent.Executors; import java.util.logging.Level; import java.util.logging.Logger; import java.text.DateFormat; +import java.util.concurrent.ThreadFactory; import net.sf.json.JSONObject; import org.kohsuke.stapler.QueryParameter; @@ -151,6 +153,11 @@ public class SCMTrigger extends Trigger { @Extension public static class DescriptorImpl extends TriggerDescriptor { + + private static ThreadFactory threadFactory() { + return new NamingThreadFactory(Executors.defaultThreadFactory(), "SCMTrigger"); + } + /** * Used to control the execution of the polling tasks. *

@@ -159,7 +166,7 @@ public class SCMTrigger extends Trigger { * of a potential workspace lock between a build and a polling, we may end up using executor threads unwisely --- they * may block. */ - private transient final SequentialExecutionQueue queue = new SequentialExecutionQueue(Executors.newSingleThreadExecutor()); + private transient final SequentialExecutionQueue queue = new SequentialExecutionQueue(Executors.newSingleThreadExecutor(threadFactory())); /** * Whether the projects should be polled all in one go in the order of dependencies. The default behavior is @@ -252,7 +259,7 @@ public class SCMTrigger extends Trigger { */ /*package*/ synchronized void resizeThreadPool() { queue.setExecutors( - (maximumThreads==0 ? Executors.newCachedThreadPool() : Executors.newFixedThreadPool(maximumThreads))); + (maximumThreads==0 ? Executors.newCachedThreadPool(threadFactory()) : Executors.newFixedThreadPool(maximumThreads, threadFactory()))); } @Override diff --git a/core/src/main/java/hudson/util/CompressedFile.java b/core/src/main/java/hudson/util/CompressedFile.java index 93c79a4d8c..231ef36857 100644 --- a/core/src/main/java/hudson/util/CompressedFile.java +++ b/core/src/main/java/hudson/util/CompressedFile.java @@ -159,7 +159,7 @@ public class CompressedFile { */ private static final ExecutorService compressionThread = new ThreadPoolExecutor( 0, 1, 5L, TimeUnit.SECONDS, new LinkedBlockingQueue(), - new ExceptionCatchingThreadFactory(new DaemonThreadFactory())); + new ExceptionCatchingThreadFactory(new NamingThreadFactory(new DaemonThreadFactory(), "CompressedFile"))); private static final Logger LOGGER = Logger.getLogger(CompressedFile.class.getName()); } diff --git a/core/src/main/java/hudson/util/NamingThreadFactory.java b/core/src/main/java/hudson/util/NamingThreadFactory.java new file mode 100644 index 0000000000..e55b7dd4a5 --- /dev/null +++ b/core/src/main/java/hudson/util/NamingThreadFactory.java @@ -0,0 +1,57 @@ +/* + * The MIT License + * + * Copyright 2013 Jesse Glick. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +package hudson.util; + +import java.util.concurrent.Executors; +import java.util.concurrent.ThreadFactory; +import java.util.concurrent.atomic.AtomicInteger; + +/** + * Thread factory that sets thread name so we know who is responsible for so many threads being created. + * @since 1.541 + */ +public final class NamingThreadFactory implements ThreadFactory { + + private final AtomicInteger threadNum = new AtomicInteger(); + private final ThreadFactory delegate; + private final String name; + + /** + * Creates a new naming factory. + * @param delegate a baseline factory, such as {@link Executors#defaultThreadFactory} or {@link DaemonThreadFactory} or {@link ExceptionCatchingThreadFactory} + * @param name an identifier to be used in thread names; might be e.g. your {@link Class#getSimpleName} + */ + public NamingThreadFactory(ThreadFactory delegate, String name) { + this.delegate = delegate; + this.name = name; // TODO consider uniquifying this + } + + @Override public Thread newThread(Runnable r) { + Thread t = delegate.newThread(r); + t.setName(name + " [#" + threadNum.incrementAndGet() + "]"); + return t; + } + +} diff --git a/core/src/main/java/jenkins/InitReactorRunner.java b/core/src/main/java/jenkins/InitReactorRunner.java index 645c185542..f3b8711e32 100644 --- a/core/src/main/java/jenkins/InitReactorRunner.java +++ b/core/src/main/java/jenkins/InitReactorRunner.java @@ -3,6 +3,7 @@ package jenkins; import hudson.init.InitMilestone; import hudson.init.InitReactorListener; import hudson.util.DaemonThreadFactory; +import hudson.util.NamingThreadFactory; import hudson.util.Service; import jenkins.model.Configuration; import jenkins.model.Jenkins; @@ -38,7 +39,7 @@ public class InitReactorRunner { es = new ThreadPoolExecutor( TWICE_CPU_NUM, TWICE_CPU_NUM, 5L, TimeUnit.SECONDS, new LinkedBlockingQueue(), new DaemonThreadFactory()); else - es = Executors.newSingleThreadExecutor(new DaemonThreadFactory()); + es = Executors.newSingleThreadExecutor(new NamingThreadFactory(new DaemonThreadFactory(), "InitReactorRunner")); try { reactor.execute(es,buildReactorListener()); } finally { diff --git a/core/src/main/java/jenkins/model/Jenkins.java b/core/src/main/java/jenkins/model/Jenkins.java index fde452c31f..eef75a402b 100755 --- a/core/src/main/java/jenkins/model/Jenkins.java +++ b/core/src/main/java/jenkins/model/Jenkins.java @@ -51,7 +51,6 @@ import hudson.model.Failure; import hudson.model.Fingerprint; import hudson.model.FingerprintCleanupThread; import hudson.model.FingerprintMap; -import hudson.model.FullDuplexHttpChannel; import hudson.model.Hudson; import hudson.model.Item; import hudson.model.ItemGroup; @@ -115,9 +114,6 @@ import static hudson.Util.fixEmpty; import static hudson.Util.fixNull; import hudson.WebAppMain; import hudson.XmlFile; -import hudson.cli.CLICommand; -import hudson.cli.CliEntryPoint; -import hudson.cli.CliManagerImpl; import hudson.cli.declarative.CLIMethod; import hudson.cli.declarative.CLIResolver; import hudson.lifecycle.Lifecycle; @@ -125,7 +121,6 @@ import hudson.logging.LogRecorderManager; import hudson.lifecycle.RestartNotSupportedException; import hudson.markup.RawHtmlMarkupFormatter; import hudson.remoting.Callable; -import hudson.remoting.Channel; import hudson.remoting.LocalChannel; import hudson.remoting.VirtualChannel; import hudson.scm.RepositoryBrowser; @@ -253,6 +248,7 @@ import javax.servlet.http.HttpServletResponse; import static hudson.init.InitMilestone.*; import hudson.security.BasicAuthenticationFilter; +import hudson.util.NamingThreadFactory; import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST; import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND; import java.io.File; @@ -282,7 +278,6 @@ import java.util.Properties; import java.util.Set; import java.util.StringTokenizer; import java.util.TreeSet; -import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.ExecutionException; @@ -3915,7 +3910,7 @@ public class Jenkins extends AbstractCIBase implements ModifiableTopLevelItemGro */ /*package*/ transient final ExecutorService threadPoolForLoad = new ThreadPoolExecutor( TWICE_CPU_NUM, TWICE_CPU_NUM, - 5L, TimeUnit.SECONDS, new LinkedBlockingQueue(), new DaemonThreadFactory()); + 5L, TimeUnit.SECONDS, new LinkedBlockingQueue(), new NamingThreadFactory(new DaemonThreadFactory(), "Jenkins load")); private static void computeVersion(ServletContext context) { diff --git a/core/src/main/java/jenkins/util/Timer.java b/core/src/main/java/jenkins/util/Timer.java index 02a4061548..a84162364b 100644 --- a/core/src/main/java/jenkins/util/Timer.java +++ b/core/src/main/java/jenkins/util/Timer.java @@ -1,11 +1,11 @@ package jenkins.util; +import hudson.util.DaemonThreadFactory; +import hudson.util.NamingThreadFactory; import javax.annotation.Nonnull; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.ThreadFactory; -import java.util.concurrent.atomic.AtomicInteger; /** @@ -42,18 +42,7 @@ public class Timer { if (executorService == null) { // corePoolSize is set to 10, but will only be created if needed. // ScheduledThreadPoolExecutor "acts as a fixed-sized pool using corePoolSize threads" - executorService = Executors.newScheduledThreadPool(10, new ThreadFactory() { - - private final AtomicInteger threadNumber = new AtomicInteger(1); - - @Override - public Thread newThread(Runnable r) { - Thread t = new Thread(r); - t.setName("Jenkins-cron-thread-" + threadNumber.getAndIncrement()); - t.setDaemon(true); - return t; - } - }); + executorService = Executors.newScheduledThreadPool(10, new NamingThreadFactory(new DaemonThreadFactory(), "jenkins.util.Timer")); } return executorService; }