Opened 18 months ago
Closed 6 months ago
#2654 closed enhancement (fixed)
PeerState.java: remove _retransmitter
Reported by: | jogger | Owned by: | zzz |
---|---|---|---|
Priority: | minor | Milestone: | 0.9.48 |
Component: | router/transport | Version: | 0.9.43 |
Keywords: | Cc: | ||
Parent Tickets: | #2649 | Sensitive: | no |
Description
Maintaining that field is pure code bloat as it simply duplicates what we have in the state pushcount. Also does not work if we want multiple retransmissions.
Subtickets
Change History (3)
comment:1 Changed 17 months ago by
Parent Tickets: | → 2649 |
---|---|
Status: | new → infoneeded_new |
comment:2 Changed 17 months ago by
Status: | infoneeded_new → new |
---|
If we have multiple retransmissions, we would record those. Code (including fast retransmission for which I still need to open a ticket) for 1) could be
int pc = state.getPushCount(); // send fresh messages if retransmissions below limit // retransmit first time if retransmissions below limit AND (fast retransmit OR time due) // send retransmitters always if time due if (_retranscount < _retransmax && (pc == 0 || pc == 1 && (state.getStartedOn() < _lastSendFullyTime || state.getNextSendTime() <= now)) || pc > 1 && state.getNextSendTime() <= now) {
As noted in #2412 I would use a getNextSendTime(), suggesting this for 2):
for (OutboundMessageState state : _outboundMessages) if (_retranscount < _retransmax || state.getPushCount() > 1) _nextSendTime = Math.min(_nextSendTime, state.getNextSendTime());
comment:3 Changed 6 months ago by
Milestone: | undecided → 0.9.48 |
---|---|
Resolution: | → fixed |
Status: | new → closed |
Fixed by #2713 in b1b866105b2533a8ccc1b0ffba02396fb581adf2 0.9.47-9
Note: See
TracTickets for help on using
tickets.
Seems to be two uses now, not convinced of "pure code bloat', please explain.
1) Is a message being retransmitted now? in locked_shouldSend(), maybe could be replaced by a boolean
2) What's the next send time, in getNextDelay(), uses the _retransmitter.getNextSendTime(), i don't see how to get rid of that. Have a proposal?
required for #2649