mina-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Emmanuel Lécharny <elecha...@gmail.com>
Subject Re: [jira] [Commented] (DIRMINA-1057) AbstractIoSession getScheduledWriteMessages always -negative?
Date Sat, 07 Oct 2017 17:35:19 GMT


Le 07/10/2017 à 16:43, Jonathan Valliere a écrit :
> Emmanuel,
>
> I’m not sure my discussions is warranted inside of the ticket unless I come
> to some sort of conclusion.

Well, we can discuss in the ticket, to keep the context around. But it's
up to you...
>
> I don’t think that is true that the message is stacked in the IoProcessor
> Queue.

I don't think I'm too :-) Actually, I re-checked teh code a bit more
throughously and you are right there is only one queue which is teh
session's queue. The IoProcessor simply loop on all the sessions that ar
eintersted in write (OP_WRITE is set) and flush the message in the socket.
>
> @Override
>     public void write(S session, WriteRequest writeRequest) {
>         WriteRequestQueue writeRequestQueue =
> session.getWriteRequestQueue();
>
>         writeRequestQueue.offer(session, writeRequest);
>
>         if (!session.isWriteSuspended()) {
>             this.flush(session);
>         }
>     }
>
> WriteRequestQueue writeRequestQueue = s.getWriteRequestQueue();
>
>             if (!s.isWriteSuspended()) {
>                 if (writeRequestQueue.isEmpty(session)) {
>                     // We can write directly the message
>                     s.getProcessor().write(s, writeRequest);
>                 } else {
>                     s.getWriteRequestQueue().offer(s, writeRequest);
>                     s.getProcessor().flush(s);
>                 }
>             } else {
>                 s.getWriteRequestQueue().offer(s, writeRequest);
>             }
>
> @Override
>     public final void flush(S session) {
>         // add the session to the queue if it's not already
>         // in the queue, then wake up the select()
>         if (session.setScheduledForFlush(true)) {
>             flushingSessions.add(session);
>             wakeup();
>         }
>     }
>
> Both use Session.getWriteRequestQueue.  Seems like the IoProcessor has a
> Queue which contains IoSession objects and not the Writable Data.

Indeed. Was too long away from the code...
> Basically, I’m looking for a concise place to unify and make the counters
> thread-safe.
Some of the counters are AtomicInt/AtomicLong ad should be thread safe.
Some are protected by a lock :

    public final void increaseScheduledWriteBytes(int increment) {
        throughputCalculationLock.lock();

        try {
            scheduledWriteBytes += increment;
        } finally {
            throughputCalculationLock.unlock();
        }
    }

but some others are simply not thread safe : writtenMessages for
instance, which is a int, incremented outside any lock.

At this point, I think we should try to be generic and only use
Atomic(Int|Long) for all the stats variables, and get rid of the locks.

wdyt ?

-- 
Emmanuel Lecharny

Symas.com
directory.apache.org


Mime
View raw message