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.

That's one of the worst reasons to make a method private. You make a method
public or protected if it may provide something usefull. You make it private
if it is strictly implementation and shouldn't be called from outside. 
There are a lot of things you need to design in order to get good 
performance, but making methods private just for that shouldn't be one of 
them.



>> 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?

Yes, they add 2 threads per webapp. Which may limit the scalability with
the number of deployed webapps ( 100 contexts -> 200 threads ).

>> 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.

It would actually be a pretty good solution IMO. A single thread checking 
the files for modification in all contexts is better ( again IMO ) than
one thread per context. Same for checking sessions. 

The coding is a bit trickier ( and there are some issues related with DOS 
and security ), but it may be worth it. 

As for someone stoping the check for class modification at run time using
an admin interface - what's wrong with that ? 


> So I'd opt for java.util.Timer. It requires J2SE 1.3+...is that a
> problem? Here's what it looks like:

What's the benefit ? More fun ? 

 
>     * 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.

Sorry, I don't think I'll go with this change.

I'm sure MonitorRunnable can be improved a lot - but for now I don't
see any bugs you'll fix or any real performance improvement.

I love refactoring code - but usually with a goal :-) 

Costin




--
To unsubscribe, e-mail:   <mailto:[EMAIL PROTECTED]>
For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>

Reply via email to