Opened 2 years ago

Last modified 2 years ago

#2647 new enhancement Do not iterate within finishmessages()

Reported by: jogger Owned by: zzz
Priority: major Milestone: undecided
Component: router/transport Version: 0.9.43
Keywords: Cc:
Parent Tickets: Sensitive: no


This looks like an important function, but those iterations do next to nothing and consume lots of CPU. The message completion code is redundant because message completion ist done by the ACK logic. Same goes for the push count check, which can never strike because we time out much earlier.

So the remaining code could be moved elsewhere or - if one was to keep the function - it would be sufficient not to iterate, but just to check the first state and possibly fail that out.


Change History (2)

comment:1 Changed 2 years ago by zzz

That's an interesting theory. The important thing here is calling _transport.succeeded() and _transport.failed(), but it does appear that those are called elsewhere too. Not clear if we're doing duplicate calls, or all the code in finishMessages() is just a failsafe, or what.

Needs further research.

comment:2 Changed 2 years ago by jogger

I have done the research = code analysis and have sucessfully run without iterations using the following on UDP only routers since more than a month.

    public int finishMessages(long now) {
        // short circuit, unsynchronized
        if (_outboundMessages.isEmpty())
            return _outboundQueue.size();

        if (_dead) {
            return 0;

        OutboundMessageState state = null;
        synchronized (_outboundMessages) {
            Iterator<OutboundMessageState> iter = _outboundMessages.iterator();
            if (iter.hasNext()) { // guard against race
                state =;
                if (state.isExpired(now))
                    state = null;
            } // end iterating over outbound messages

        if (state != null) {
            if (state.getPushCount() > 1)
            OutNetMessage msg = state.getMessage();
            if (msg != null) {
            } else {
                // it can not have an OutNetMessage if the source is the
                // final after establishment message
                if (_log.shouldLog(Log.WARN))
                    _log.warn("Unable to send a direct message: " + state);

        // no need to update _nextSendTime as we will immediately
        // either remove() or call allocateSend()
        return _outboundMessages.size() + _outboundQueue.size();
Note: See TracTickets for help on using tickets.