[concurrency-interest] DefaultThreadFactory implementation questions

Tim Peierls tim at peierls.net
Wed Jan 14 13:40:45 EST 2009


On Wed, Jan 14, 2009 at 12:15 PM, Roel Spilker <R.Spilker at topdesk.com>wrote:

>  Regarding (1):
>
> The constructor public Thread(Runnable target, String name) already uses
> the ThreadGroup of the SecurityManager by providing null as the ThreadGroup
> to the init method.
>

As the code stands, you get the ThreadGroup of the current
SecurityManager (or Thread) when the DefaultThreadFactory is constructed
rather than whatever is current when newThread is called. If the code were
to use null for its ThreadGroup parameter (explicitly or implicitly), it
would get the latter behavior. The former behavior is better because the
user controls (or at least knows) what SecurityManager and Thread are
current when the factory is created. It isn't specified what SecurityManager
and Thread are current when newThread is called; it might be different for
each call.



Regarding (2):
>
> I agree you could do it like that. However, I think it would still be a
> good idea to make it easy for the users to create a default thread factory
> with a given namePrefix. If we would follow your reasoning, there is also no
> need for public static ExecutorService newFixedThreadPool(int nThreads),
> since the caller could call public static ExecutorService
> newFixedThreadPool(int nThreads, ThreadFactory threadFactory) after getting
> threadFactory using Executors.defaultThreadFactory().
>

None of the methods in Executors are absolutely essential, but they provide
often-needed j.u.c-related functionality that reduces code verbosity in
common cases and/or is difficult to get right from scratch. I don't think
adding prefixes to thread names is either that common or that difficult to
get right. And if you use a decorator of the sort that I described, it
needn't be verbose, either.

In addition, I think it's much nicer to readers of your code not to require
esoteric knowledge of library parameters.

  ExecutorService ex = newFixedThreadPool(2, "Main"); // Quick, what does
this mean? Or was it ("Main", 2)?

might be compact, but it isn't as friendly as this:

  ExecutorService ex = newFixedThreadPool(2,
withNamePrefixedBy("Main", defaultThreadFactory()));




> If "folks would disagree on the precise format", they could always choose
> to use a decorator. But providing a default implementation that enhances
> debugging and monitoring consists of a simple API change, and requires only
> a few lines of code to be changed. I think that would be good value for
> money.
>

I don't think it pulls more than its throw weight (if I have the metaphor
right). It would clutter the Executors class with methods with magic
parameters in order to elevate just one of many valid custom thread factory
use cases, one that happens to be easy to implement by hand in a few lines
of code. And these methods wouldn't be universally used even by those who
wanted them because no one would agree on exactly how to do it, e.g.,
whether to follow the prefix with a hyphen -- so half of the potential users
would roll their own anyway.

--tim
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://cs.oswego.edu/pipermail/concurrency-interest/attachments/20090114/929f9769/attachment.html>


More information about the Concurrency-interest mailing list