Rob,
On 3/19/07, Rob Butler <[EMAIL PROTECTED]> wrote:
In my opinion ProtocolDecoders should be stateless and a single instance should
be used for all sessions. This allows multiple sessions to be handled with a
single ProtocolDecoder instance.
It is probably far more common for protocols to be stateless. Note I am
talking about the communication PROTOCOL. Clients and servers may be stateful,
but the communication protocol even for these systems usually is not.
Stateful ProtocolDecoders can be implemented very simply by storing state in the session
as an attribute. Even if the protocol must implement a complex state machine with
multiple attributes, this is best done by storing a single "state" object in
the session and using the State Pattern.
I feel implementing stateful ProtocolDecoders by instantiating a new one for
each session and storing state within the ProtocolDecoder itself is a very bad
idea.
At the very least, please make sure that Mina doesn't always instantiate a new
ProtocolDecoder instance in support of stateful ProtocolDecoders. This will
add unnecessary overhead to servers where the protocol is not stateful.
If your ProtocolDecoder is implemeted stateless, your ProtocolCodecFactory
could always return the same instance, and the performance overhead will
be nihil.
Maarten
----- Original Message ----
From: Maarten Bosteels <[EMAIL PROTECTED]>
To: [email protected]
Sent: Sunday, March 18, 2007 3:44:58 PM
Subject: Re: ProtocolDecoderAdapter and session-state.
On 3/17/07, Niklas Therning <[EMAIL PROTECTED]> wrote:
> Maarten Bosteels wrote:
> > Hi Niklas,
> >
> > I am reading the mina mailing list since about a year, but this is the
> > first time I see a recommendation to keep the ProtocolDecoder
> > implemantation stateless.
> > I prefer to create a new decoder per session and keep the decoder
> > state in the decode.
> > Sound OO-principle says : store the state in the class that uses it.
>
> Maybe stateless was the wrong word here. There's nothing wrong with
> storing state in your ProtocolDecoder implementation. As long as it
> isn't per-session state.
I was talking about per-session state: the state of the decoding
algorithm for one message.
For example: suppose each message starts with a length-header (an integer),
once the header is read, I would store the length of the incomplete message
as an instance variable of the decoder, because the decoder is the
only one that needs to know this data. WDYT ?
I guess James is right about the visibility issue, hadn't thought
about that yet,
but I can solve that without storing the state in the IoSession.
> Even if that will work in MINA 1.0 I guess that
> could change in future versions. AFAIK there's nothing in the docs that
> tells you that MINA 1.0 creates one decoder per session, right? You
> would have to look at the ProcolCodecFilter code.
You are right, some time ago I already noticed that it wasn't in the
docs, and I had plans to raise it as an issue.
http://mina.apache.org/report/trunk/apidocs/org/apache/mina/filter/codec/ProtocolCodecFactory.html#getDecoder()
says:
"Returns a new (or reusable) instance of ProtocolEncoder which encodes
message objects into binary or protocol-specific data."
I think it should be documented that this method is called once for
each new session, no ?
And maybe, the docs could also mention that storing per-session state
in the decoder isn't a best-practice, although I am not yet sure why
not :-)
Maarten
>
> I think that if this is something users do and expect to work we need to
> enforce this rule (one decoder instance per session) somehow to prevent
> incompatibilities later on. At least document it thoroughly.
>
> >
> > Should I really anticipate that mina will someday no longer support
> > statefull decoders ?
>
> No, you shouldn't. :-) I'm just saying that AFAIK there's really nothing
> that says that it works that way and at the moment you cannot expect it
> not to change in the future. It's great that you're raising the issue
> because if this is what users want we need to make sure it stays that way.
>
>
> --
> Niklas Therning
> www.spamdrain.net
>
>
____________________________________________________________________________________
TV dinner still cooling?
Check out "Tonight's Picks" on Yahoo! TV.
http://tv.yahoo.com/
