Implemented Broadcast Options for NonBlockingSocket
#67
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Objective
NonBlockingSocket
for Message Broadcasts #45Solution
I've added additional methods to
NonBlockingSocket
for more specific forms of message transmission:send_to_many
: Send a single message to multiple recipients (single multicast)send_many_to
: Send multiple messages to a single recipient (batched singlecast)send_many_to_many
: Send multiple messages to multiple recipients (batched multicast)Since these methods can be defined in terms of repeated
send_to
calls, default implementations are provided for all three, ensuring no breaking change in the external API. What this does allow is for implementers ofNonBlockingSocket
to create optimised transmission behaviour for certain cases.To utilise these new transmission options, I've done some refactoring of
UdpProtocol
. First, I've introduced aBROADCAST
MessageHeader
magic
value, which will allow endpoints to accept broadcasted messages. (currently, every message includes a connection-specific magic number which makes broadcast infeasible. By adding a broadcast special value, the check can be bypassed).Next, I refactored the input and checksum senders to instead clear the pending message queues, and then broadcast. I clear the message queues first to ensure the order of messages is preserved. Broadcast is then done immediately, since I can't think of a reasonable design change to allow a mixture of singlecast and broadcast messages across all connections.
Finally, I changed
send_all_messages
to actually send all messages usingsend_many_to
. If a socket implementation has a way of batching messages together effectively, this change would be heavily utilised.Some other misc. changes:
HandleMessage
trait to try and split out some of the code out of the monolithicUdpProtocol
implementation block. It makes no difference to calling behaviour, I just personally think it organises that part of the code a little better.UdpProtocol::send_queue
to aVec
instead of aVecDeque
. There were no instances ofpop_front
orpush_front
, so no functionality is lost. The benefit of this change is it allows for getting a contiguous slice of messages directly without collection.&mut Box<dyn NonBlockingSocket<T::Address>>
in function signatures with&mut (impl NonBlockingSocket<T::Address> + ?Sized)
. While this looks similar (and can be used almost interchangeably), it actually allows the compiler to perform monomorphization at compile time, since the type implementingNonBlockingSocket
is always known at compile time. I doubt this translates to much performance difference in reality, but it is generally a good practice in my experience. In this case, I believe it will be monomorphed aroundBox<dyn NonBlockingSocket<T::Address>>
anyway, so at worst there's no change.Notes
Currently,
send_many_to_many
is unused by GGRS in this PR, as I could not find any instances of sending the same batch of messages to the same list of peers. However, it's such a natural consequence of adding batched singlecast and single multicast, it feels incomplete to leave it out. Perhaps in the future there may be a use for it?Additionally, in the
UdpProtocol::send_input_to_many
method, I check that all input packets actually are the same before usingsend_to_many
, falling back to repeatedsend_to
on difference. I'm not confident enough to refactor how the input messages are created to ensure they are all identical, or have the differing parts could be segregated out, but I do think that's something to look into for future performance boosts.Finally, I haven't changed the implementation of
NonBlockingSocket
forUdpNonBlockingSocket
, since I don't know if for UDP packets there's a way to get substantially more efficiency with batching up the messages. It might, but testing would be required to justify such a change. For more bespoke implementation like in #45 however, I think the changes would be substantial.