On 2/22/10 11:22 AM, Ashish wrote:
On Mon, Feb 22, 2010 at 3:14 PM, Julien Vermillard
<[email protected]> wrote:
The idea of doing that in a (optional) parallel thread, will generate a
CPU peak only each seconds (as actual code) but will avoid calling
System.currentTimeMillis() after every select() call.
BTW we already got an Idle checker based on thread used for serial
transport and VMPipe.
It'll be lighter in code and CPu, but at the cost of a thread for each
IoService.
IMO I think it worth the cost.
Sorry, but taking a step back, so that I understand things :)
1. What's the Cost of calling System.currentTimeMillis()? Is it too costly..
On my computer, I can do 100 000 000 call in 5 seconds, rouglhy 50 ns
each. Not expensive.
However, if you look at the current code, we aren't doing only a single
call to this method when dealing with idleness. The mail select() loop
calls the notifyIdleSessions(currentTime) each time the select() exits,
so either because we have had sometng to read or write, or when nothong
happened for one second (this is not configurable, btw).
What happens when this method is called ? Well, if it has been called
before one full second elapsed, then a simple test is done and we don't
do anything but this test in this case. We could perfectly be waiting
almost 2 seconds in some cases (if the select() is waken just before 1
second, say 999 ms, and then goes back to sleep for 1 full second : as
the first delay is less tahn one second, we do nothing, and as we have
to wait 1 more second in the select, we don't detect idle session for
1.999 ms).
Now, let's say that we have waited for one full second. The method does :
private void notifyIdleSessions(long currentTime) throws Exception {
// process idle sessions
if (currentTime - lastIdleCheckTime >= SELECT_TIMEOUT) {
lastIdleCheckTime = currentTime;
AbstractIoSession.notifyIdleness(allSessions(), currentTime);
}
}
Here, we grab *all* the sessions registred on the current selector (it's
an iterator) and we call the notifyIdleness method, which calls the
notifyIdleSession() method for every session :
public static void notifyIdleness(Iterator<? extends IoSession>
sessions, long currentTime) {
IoSession s = null;
while (sessions.hasNext()) {
s = sessions.next();
notifyIdleSession(s, currentTime);
}
}
and :
/**
* Fires a {...@link IoEventType#SESSION_IDLE} event if applicable for the
* specified {...@code session}.
*
* @param currentTime the current time (i.e. {...@link
System#currentTimeMillis()})
*/
public static void notifyIdleSession(IoSession session, long
currentTime) {
notifyIdleSession0(
session, currentTime,
session.getConfig().getIdleTimeInMillis(IdleStatus.BOTH_IDLE),
IdleStatus.BOTH_IDLE, Math.max(
session.getLastIoTime(),
session.getLastIdleTime(IdleStatus.BOTH_IDLE)));
notifyIdleSession0(
session, currentTime,
session.getConfig().getIdleTimeInMillis(IdleStatus.READER_IDLE),
IdleStatus.READER_IDLE, Math.max(
session.getLastReadTime(),
session.getLastIdleTime(IdleStatus.READER_IDLE)));
notifyIdleSession0(
session, currentTime,
session.getConfig().getIdleTimeInMillis(IdleStatus.WRITER_IDLE),
IdleStatus.WRITER_IDLE, Math.max(
session.getLastWriteTime(),
session.getLastIdleTime(IdleStatus.WRITER_IDLE)));
notifyWriteTimeout(session, currentTime);
}
private static void notifyIdleSession0(
IoSession session, long currentTime,
long idleTime, IdleStatus status, long lastIoTime) {
if (idleTime > 0 && lastIoTime != 0
&& currentTime - lastIoTime >= idleTime) {
session.getFilterChain().fireSessionIdle(status);
}
}
private static void notifyWriteTimeout(
IoSession session, long currentTime) {
long writeTimeout = session.getConfig().getWriteTimeoutInMillis();
if (writeTimeout > 0 &&
currentTime - session.getLastWriteTime() >= writeTimeout &&
!session.getWriteRequestQueue().isEmpty(session)) {
WriteRequest request = session.getCurrentWriteRequest();
if (request != null) {
session.setCurrentWriteRequest(null);
WriteTimeoutException cause = new
WriteTimeoutException(request);
request.getFuture().setException(cause);
session.getFilterChain().fireExceptionCaught(cause);
// WriteException is an IOException, so we close the
session.
session.close(true);
}
}
}
Note that for every idle session, we jump into the chain to propagate
the IdleSession event, and we potentially do it 3 times. This could
probably be done only once.
However, the main problem here is that if we have thousands of idle
session, then we will pump CPU time from the main select() thread to
handle them, while active sessions will be waiting until this is done.
I really think that a separated thread could be more efficient and fair...
2. Is the discussion converging that we update a boolean flag upon
each access and use that to determine if the session is stale.
Julien, will it be possible for you to elaborate a bit on how is it
lighter in code& CPU. I don't have insight into these two modules.
May be using a simple boolean flag could be the wrong approach, as it's
damn too simple. The current MINA code make it possible to manage an
idle delay per session, as soon as you configure it. I'm pretty sure
that many would like to keep this possibility.
Also note that this idleness is used to detect slow readers, as we close
sessions when we don't have reads for a long time while we have some
data waiting to be read.
--
Regards,
Cordialement,
Emmanuel Lécharny
www.nextury.com
