tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bill Barker" <wbar...@wilshire.com>
Subject Re: [PATCH] Re: ThreadPool
Date Sat, 28 Dec 2002 10:03:11 GMT

----- Original Message -----
From: "Remy Maucherat" <remm@apache.org>
To: "Tomcat Developers List" <tomcat-dev@jakarta.apache.org>
Sent: Saturday, December 28, 2002 12:40 AM
Subject: Re: [PATCH] Re: ThreadPool


> Michael wrote:
> >  Costin Manolache wrote:
> >
> >> You're probably right that addThread() doesn't need to be public - but
> >> I don't think it hurts too much either.
> >
> >
> > It does when you're trying to refactor a class. It forces you to ask
> > whether the interface really is used, or can you rearrange like you want
> > to. Usually I grep the sources to determine that, but when you're using
> > reflection/introspection (e.g., JMX), then there's no telling. You have
> > to take an educated guess and hope that you don't break things.
> >
> > It also prevents the byte code compiler from doing complete, inlined
> > optimization when a member is non-private.
> >
> >>> Also: MonitorRunnable, now that it has a start() method (for JMX, I
> >>> presume) has no restriction on running more than one thread. This
> >>> undercuts the meaning of "interval", and makes stop() poorly defined.
> >>
> >>
> >>
> >> IMO MonitorRunnable, the session expiration thread ( one per ctx )
> >> and the reloading thread ( one per ctx again )
> >
> >
> > What classes are you referring to specifically? StandardManager &
> > WebappLoader?
> >
> >> should all go away and
> >> be replaced by a single mechanism. Maybe the JMX timer ?
> >
> >
> > I'd hate to think what would happen if someone stopped the timer
> > service. Also, using the event-driven approach to trigger maintenance
> > activities in foreground threads would be quirky coding, and may not
> > even be appropriate in classes that don't always have executing code.
> >
> > So I'd opt for java.util.Timer. It requires J2SE 1.3+...is that a
> > problem? Here's what it looks like:
> >
> >    * Replace MonitorRunnable with java.util.Timer.
> >    * The Timer thread is always daemonized; I couldn't understand why
> >      MonitorRunnable would conditionally do so based on ThreadPool.
> >    * Unlike the start/stop() MonitorRunnable i/f--which I assumed was
> >      for activating the new wait() interval--the Timer can never be
> >      stopped w/o shutting down the ThreadPool.
>
> If I were you, I would pick a less critical section to start playing
> with, so that we get to know your code, if you know what I mean.
>
> Also, I would like it if this was fixing any issue or actually improving
> performance, rather than just beautifying the code, which is not
desirable.

Not to mention that this section of code is used by TC 3.3.  I'll '-1' the
patch for that reason, since it won't work for a 1.1.x JVM (as TC 3.3
currently requires).

>
> Remy
>
>
> --
> To unsubscribe, e-mail:
<mailto:tomcat-dev-unsubscribe@jakarta.apache.org>
> For additional commands, e-mail:
<mailto:tomcat-dev-help@jakarta.apache.org>
>


--
To unsubscribe, e-mail:   <mailto:tomcat-dev-unsubscribe@jakarta.apache.org>
For additional commands, e-mail: <mailto:tomcat-dev-help@jakarta.apache.org>


Mime
View raw message