Opened 3 weeks ago

Last modified 5 days ago

#2647 new enhancement

PeerStata.java: 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

Description

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.

Subtickets

Change History (2)

comment:1 Changed 7 days 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 5 days 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) {
            dropOutbound();
            return 0;
	    }

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

        if (state != null) {
            if (state.getPushCount() > 1)
                _retranscount--;
            OutNetMessage msg = state.getMessage();
            if (msg != null) {
                _transport.failed(state);
            } 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.