mina-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Emmanuel Lécharny <elecha...@gmail.com>
Subject MINA 2 : a few things we can do...
Date Thu, 08 Nov 2012 17:37:20 GMT


Hi guys,

we have spent some extensive time with Julien during the Apache
Conference disucssing about MINA 3, and analysing some parts of MINA 2
code. There are a few things we could probably fix in MINA 2 that would
speed up a bit the way it works, mainly for TCP  :

1) currently, when we write a message which goes throgh the
PrococolCodecFilter, we generate an empty message which is used to
trigger the messageSent event. The reason is that we may generate more
than one IoBuffer while encoding the message (this is a very doubtful
assumption though : why on earth would be generate more than one
IoBuffer ???). I do think we have better ways to get the messageSent
event being generated. Here is some idea :

- we get back a queue of messages from the encoder :

ProtocolCodecFilter.filterWrite() {
    ...
    // Now we can try to encode the response
    encoder.encode(session, message, encoderOut);

    // Send it directly
    Queue<Object> bufferQueue = ((AbstractProtocolEncoderOutput)
encoderOut).getMessageQueue();

    // Write all the encoded messages now
    while (!bufferQueue.isEmpty()) {
        Object encodedMessage = bufferQueue.poll();

        if (encodedMessage == null) {
            break;
        }

        // Flush only when the buffer has remaining.
        if (!(encodedMessage instanceof IoBuffer) || ((IoBuffer)
encodedMessage).hasRemaining()) {
            SocketAddress destination = writeRequest.getDestination();
            WriteRequest encodedWriteRequest = new
EncodedWriteRequest(encodedMessage, null, destination);

            nextFilter.filterWrite(session, encodedWriteRequest);
        }
    }

    // Call the next filter
    nextFilter.filterWrite(session, new MessageWriteRequest(writeRequest));
    ...

Here, we can see that for each encoded IoBuffer, we call the
nextFilter.filterWrite() method, and when the queue is exhausted, we
call it again with an empty message. I don't think to be wrong in saying
that 100% of the time, we don't have more than one IoBuffer to write.

Why do we need this empty message then ? Because otherwise, the
messageSent event will never be forwarded to the IoHandler :

DefaultIoFilterChain.fireMessageSent(WriteRequest request) {
        ...
        if (!request.isEncoded()) {
            callNextMessageSent(head, session, request);
        }
    }

the request.isEncoded() method always returns true when the message is
an EncodedWriteRequest instance :

    private static class EncodedWriteRequest extends DefaultWriteRequest {
        public boolean isEncoded() {
            return true;
        }

At this point, I think we can safely remove this empty message, still
keeping the same behavior (ie, the messageSent event will only be sent
when the last message in the codec queue is sent) if we add a flag in
the EncodedWriteRequest instance that says that this is the last
message. We will also need to inject the original message into this data
structure.

The gain is not enormous, but still, it takes time and memory to process
this empty message.

2) We have another issue we can work on : right now, when we do a
session.write(),it always ends up with some IoBuffer being pushed into a
queue, for the IoProcessor to read this queue and send the message
later. This
can be a huge problem if the user is getting back a WriteFuture like this :

        WriteFuture future = session.write(message);
       
        future.awaitUninterruptibly();

because this will block forever. This is very confusing, as users will
expect the message to be sent when it's written, and expect the server
to return when it's considered as sent. Actually, the message is just
enqueued when the session.write() method is called, and the WriteFuture
is only signaled when the message has been fully written *in the
socket*. But the current thread won't be able to write the message into
the socket because it's blocked waiting for the message to be written !

The way the IoProcessor works make it impossible to use such code :

            for (;;) {
                try {
                    ...
                    int selected = select(SELECT_TIMEOUT);  <----------
Here we received some data
                    ...
                    if (selected > 0) {
                        process();  <---------- We have to process those
data
                    }
                    ...
                   
    private void process() throws Exception {
        for (Iterator<S> i = selectedSessions(); i.hasNext();) {
            S session = i.next();
            process(session);  <---------- We process the data for the
specific session
            i.remove();
        }
    }
   
   
    private void process(S session) {
        // Process Reads
        if (isReadable(session) && !session.isReadSuspended()) {
            read(session);  <---------- We read teh data and call the
messageReceived() message, which goes up to the IoHandler
        }

        // Process writes
        if (isWritable(session) && !session.isWriteSuspended()) {
            // add the session to the queue, if it's not already there
            if (session.setScheduledForFlush(true)) {
                flushingSessions.add(session);  <---------- Now we
inform the processor that we have something to write
            }
        }

As we can see in the last method, the writes can only be processed after
the read is completed, something that will never occur if the read is
blocked waiting for the write to be done...

This is a really bad situation. Not only we block the server for this
session, but we make the user think that it's the way he should write
the IoHandler code...

Sadly, there is little we can do to fix this issue as t will break the
API badly. That makes the WriteFuture totally useless, and it put the
server at risk to quickly get some OOM as every message is pushed into
an internal queue until the thread is freed.

So what can we do ?

There is one fix that can help, but it's not an ultimate solution. In
MINA 3, we should do something totally different.

The idea is to try to write in the socket immediately, instead of
transiting through a queue. Here there are three cases :
- the socket can be written, and can contain the whole message :
    We are safe, the WriteFuture will immediately return.
- the socket can be written, but it will not accept the full message :
    that's a problem, because the writeFuture will hang forever.
- the socket is already full. Same problem here : we will hang.

As we can see, we may get one case where the writeFuture will not block,
but there is no solution in the second case...

That let's us with one solution : the WriteFuture should *never* block.
We have to deprecate the await() methods, and clearly inform the users
not to call them.

Now, if we write directly into the socket, we will save some CPU and
some memory, as we won't have anymore to enqueue the message. We will
still enqueue it if it can't be sent because the sockaet has blocked or
is blocked.

I suggest we do something totally different : we should *never* process
a message in the selector's thread. If we have two separate threads, one
for the selector, one for the processor, then the processor can push the
message into a queue, set the selectionKey flag to OP_WRITE and wake up
the selector. This is simple, the WriteFuture will work accordingly to
the API, and we won't have any blockage.

That means we have to create one separated worker thread in the
IoProcessor to process the messages. This is doable without breaking the
API, I think.
                   

Anyway, I will try to work around those ideas in the next few days to
see if it fits.

In MINA 3, we should be extremelly careful not to reproduce such
behaviors. Hopefully, Julien is doing an excellent job here, so I'm
confident that MINA 3 will be way better !

One last note : if you have injected an Executor in the chain, then
suddenly, the writeFuture will not block anymore, because the main
thread ha sbeen released and can process the write while the IoHandler
thread is waiting for the write to be done... But adding an executor in
the chain creates more problem than it solves here...

-- Regards, Cordialement, Emmanuel Lécharny www.iktek.com


-- 
Regards,
Cordialement,
Emmanuel Lécharny
www.iktek.com 


Mime
View raw message