Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implemented Broadcast Options for NonBlockingSocket #67

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bushrat011899
Copy link

Objective

Solution

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 of NonBlockingSocket 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 a BROADCAST 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 using send_many_to. If a socket implementation has a way of batching messages together effectively, this change would be heavily utilised.

Some other misc. changes:

  • I've added a small (and private) HandleMessage trait to try and split out some of the code out of the monolithic UdpProtocol implementation block. It makes no difference to calling behaviour, I just personally think it organises that part of the code a little better.
  • I changed UdpProtocol::send_queue to a Vec instead of a VecDeque. There were no instances of pop_front or push_front, so no functionality is lost. The benefit of this change is it allows for getting a contiguous slice of messages directly without collection.
  • I've replaced uses of &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 implementing NonBlockingSocket 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 around Box<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 using send_to_many, falling back to repeated send_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 for UdpNonBlockingSocket, 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow Optimizing NonBlockingSocket for Message Broadcasts
1 participant