----- Original Message ----- From: "Remy Maucherat" <[EMAIL PROTECTED]> To: "Tomcat Developers List" <[EMAIL PROTECTED]> 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:[EMAIL PROTECTED]> > For additional commands, e-mail: <mailto:[EMAIL PROTECTED]> > -- To unsubscribe, e-mail: <mailto:[EMAIL PROTECTED]> For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>
