> even the more important 'Interface for resettable,
> time-guarded, operations' issue has not been resolved.
I've been distracted on making sure that at least the path through
SMTPHandler, up to the point where it hands off to the spooler, is clean.
At the moment, I have a 12 hour run going with heap profiling turned on. It
appeared to be clean in a prelimnary heap snapshot last night. Also found a
few places where we can significantly reduce memory footprint, and cut down
on GC. The code running in this test is at least 33% faster than the code
previously reported on the October 18th, based upon my re-testing both SAR
files under the same conditions over hours long tests. YMMV.
> I would have preferred to have a better scheduler implementation
> in place till the 'resettable, time-guarded' proposal is resolved.
Seems to me that you owed me a revised SAR file for testing. But don't
worry about it. I realized that it would be straightforward to wrap an
Avalon Scheduler inside a Watchdog interface. That basically has the same
impact, and pretty much the same signatures, as the earlier patch that
didn't make it into BaseConnectionHandler. The handler would be isolated
from the implementation.
> we should not be refactoring existing stable code and
> pulling in parts of Avalon unless we need to.
James is an Avalon application. My own opinion is that if James is going to
use additional packages, there should be a good reason not to use one
provided by Avalon; why create more footprint than we need? If there are
problems with the Avalon components, that should be communicated back to the
Avalon group. That said, there will be cases where other packages make
sense. The Watchdog is an example.
> I think is better to keep the current abstraction as
> there is lot of commonality between protocol handlers.
What "abstraction" do you have in mind? What "commonality" of *INTERFACE*
do protocol handlers have that isn't provided by the ConnectionHandler
interface provided by Avalon? BaseConnectionHandler provides common
IMPLEMENTATION, which should not be exposed outside of the handlers.
Now that you've gotten me to take a closer look at the object relationships,
I notice that BaseConnectionHandler's interface is:
BaseConnectionHandler extends AbstractLogEnabled implements Configurable
BaseConnectionHandler is a misnomer; it is NOT a ConnectionHandler. THAT
interface is introduced by each specific Handler, e.g.,
public class SMTPHandler
extends BaseConnectionHandler
implements ConnectionHandler, Composable, Configurable, Target
So what is in BaseConnectionHandler? Configuration data:
private static int DEFAULT_TIMEOUT = 1800000;
protected int timeout = DEFAULT_TIMEOUT;
protected String helloName;
public static String configHelloName(final Configuration configuration)
public void configure( final Configuration configuration )
public void releaseConnectionHandler(ConnectionHandler
connectionHandler)
None of that is necessary after making the change that you, yourself,
suggested, which is to have a single, shared, pre-prepared, service-wide
configuration object that is given to the handler. The release method isn't
used anywhere within James, which makes sense since in simple point of fact,
there is not one line of code in James, other than the "extends" clause for
each handler, that references BaseConnectionHandler.
> variables like Socket, Input Stream, Output Stream,
> etc. need not exist in separate handlers.
This is an implementation issue, not an abstraction, and not even present in
the current BaseConnectionHandler. For example, since
handlerConnection(Socket) is abstract in BaseConnectionHandler, I might
argue that it breaks encapsulation to put the instance variable in the
abstract class. The same is true of the values currently in
BaseConnectionHandler. As you suggested, amongst others, handler
configuration is a shared object that is passed to each handler by the
HandlerFactory. So the timeout values for that service are shared. The
hello name for that service is shared. Etc.
BaseConnectionHandler is a misnomer, it is not part of the abstraction (as
seen in the code), and it is vestigial. Therefore it is unnecessary now,
and should there be some common implementation determined in the future, it
could be added by either inheritance or delegation without changing the
abstraction.
--- Noel
--
To unsubscribe, e-mail: <mailto:james-dev-unsubscribe@;jakarta.apache.org>
For additional commands, e-mail: <mailto:james-dev-help@;jakarta.apache.org>
