Thank you for comments, toad! Just a bit of my motivation for each point: >> We do not send anything until we can send a full packet.
Even if the packet is already overdue? Yeah, I understand the motivation for coalescing, but the previous code did send the packet even if there's no fullPacketQueued() in case it found any which deadline is already past now. There was a separate if-branch for it in the algo. The current coalesce-until-we-can seems more reasonable in many cases. Am I missing something or it was also a bug? >> IMHO what we need to do here is always go back around the loop after sending a packet. Your code eventually had a separate loop which re-queries each node; easier to use the real logic IMHO. We can't just check the peer we sent on because others might have it, as you discovered; and going back over all the peers is IMHO ugly, easier to use the same code we use for selecting a peer to send to in the first place, fewer special cases. Yes, general code re-use is an awesome thing, but in this concrete case it completely ruins the transparency of logic - those checks will be distributed across the whole realRun(), which is already kinda huge. Understand all the picture of when and by what concrete decision in the algorithm the actuall change to nextActionTime in the given situation is applied is increasingly hard. That's one of the main reasons why it took so much time to examine it (of course, except for the fact that this bug could reside in any of the classes up-the-hierarchy). Moreover, even if not count the bugs already mentioned, the previous approach led to the other one (fixing which will mix up the logic even further) - just a simple example to be clear: imagine that it has sent the packet with the deadline 10ms from now to the node A. The following packet in it's queue has the deadline of 20ms. The queue of the node B consists of only one packet with deadline 15ms. So, since it only checks the node's A packetqueue once again after the packet is sent it will either have to wake up in 10ms (deadline of the sent packet) with no reason to actually do so, either wait() 20ms and skip the B's deadline. So, in any case, either it should query all the nodes once again (which in turn is much more transparent and compact), either track the second best result (which is less expandable and more obscure). Thanks again! quadrocube _______________________________________________ Devl mailing list [email protected] https://emu.freenetproject.org/cgi-bin/mailman/listinfo/devl
