Niklas Therning wrote:
> Trustin Lee wrote:
>> Hi Niklas,
>>
>> On 4/26/06, Niklas Therning <[EMAIL PROTECTED]> wrote:
>>> Trustin Lee wrote:
>>>> On 3/27/06, Niklas Therning <[EMAIL PROTECTED]> wrote:
>>>>
>>>>> In protocols like SMTP when there are simple line-based commands
>>>>> intermixed with raw data (mail data) there are also great opportunities
>>>>> for optimization if you write your own codec filter. I've
>>> implemented my
>>>>> own DecoderFilter which can operate in "data mode". When not in data
>>>>> mode the filter will act more or less like an ordinary ProtocolDecoder,
>>>>> copying the received buffer to an autoexpanding buffer, decode SMTP
>>>>> commands and pass them on to the next filter. When in data mode however
>>>>> the filter will simply forward the buffers as they are received without
>>>>> any copying (in most cases).
>>>>>
>>>>> I guess this could be achieved with the MINA codec package but not
>>>>> without some tweaking and not as efficiently. Please let me know if I'm
>>>>> wrong.
>>>>>
>>>>> I wouldn't mind adding this filter to MINA if anyone is interested.
>>>>
>>>>
>>>> It would be nice if we can generalize this behavior. We could then
>>> switch
>>>> arbitrary set of filters in runtime fairly easily. WDYT?
>>>>
>>>> Trustin
>>> I think DIRMINA-201 solves the efficiency issue I was referring to. The
>>> only thing that would have to change to make ProtocolCodecFilter suit my
>>> needs is that SimpleProtocolDecoderOut shouldn't queue messages but
>>> rather forward them to the nextFilter right away as they are written.
>>> Then my IoHandler would be able to instruct my Decoder instance to
>>> change its state and decode differently depending on what the previous
>>> message was.
>>
>> If we directly call the IoHandler.messageReceived() when we call
>> ProtocolDecoderOut.write(), any exceptions raised by the handler can
>> interrupt the decoding process. SimpleProtocolDecoderOut.write() could
>> catch the exception but I think it cannot fire exceptionCaught event
>> correctly because it cannot access AbstractIoFilterChain without breaking
>> the OO design. Moreover, we cannot guarentee that
>> IoHandler.messageReceived()
>> is executed in the same thread with ProtocolDecoderOut.write() because
>> there can be an extra thread pool between the codec and the handler.
>>
>> Is it a problem for you to change the decoder state in the decoder itself?
>
> I'd rather keep my decoder/encoder oblivious of state. I'd like to keep
> them as simple as possible.
>
> Here's what I propose: make the ProtocolDecoderOut used by
> ProtocolCodecFilter pluggable and make the default behave exactly as
> today. Add the method flush() to ProtocolDecoderOut. Change
> ProtocolCodecFilter.messageReceived() slightly:
>
> ProtocolDecoderOutput decoderOut = getDecoderOut(nextFilter,session);
>
> try
> {
> decoder.decode( session, in, decoderOut );
> }
> catch( Throwable t )
> {
> ... // No change here
> }
> finally
> {
> // Dispose the decoder if this session is connectionless.
> ...
>
> // Release the read buffer.
> in.release();
>
> decoderOut.flush();
> }
>
> The code
> Queue queue = decoderOut.getMessageQueue();
> while( !queue.isEmpty() )
> {
> nextFilter.messageReceived( session, queue.pop() );
> }
> would move into the flush() method of SimpleProtocolDecoderOutput.
>
> Now I could extend ProtocolCodecFilter and override getDecoderOut() to
> use my non-queuing PDO instead. It would look something like:
>
> public class NonQueingProtocolDecoderOut implements ProtocolDecoderOutput
> {
> private final IoSession session;
> private final NextFilter nextFilter;
> public NonQueingProtocolDecoderOut(IoSession session, NextFilter
> nextFilter)
> {
> this.session = session;
> this.netxFilter = nextFilter;
> }
>
> public void write( Object message )
> {
> nextFilter.messageReceived( session, message );
> }
>
> public void flush() {
> }
> }
>
> This solution avoids calling IoHandler.messageReceived() directly. It
> uses the NextFilter which will handle the exceptionCaught() as expected
> so the exception will never be thrown into the PDO.
>
> The threading issue is of course still a problem but that is something
> we would need to document. It doesn't make any sense to add a
> ThredPoolFilter after the ProtocolCodecFilter if you want to achieve
> what I want to.
>
> I guess the flush() method should only be called by MINA so adding that
> to the interface could be a problem. But it could also be a feature? I
> could actually achieve what I want by calling flush from my decoder with
> the SimpleProtocolDecoderOutput!
>
> WDYT?
>
If no one objects I will go ahead and make these changes.
One concern I have is with the NextFilter. Right now the PDO is being
cached in the session. Using the approach I described above it will hold
on to the NextFilter instance. Is that safe? Is there any reason why we
can't create a new PDO each time we need one instead of caching one in
the session?
--
Niklas Therning
Software Architect
www.spamdrain.net
