ENet packets sent with put_packet() are not aggregated (and result in higher packer overhead)

[dreadpon]

Godot version

3.5.stable

System information

Windows 10, Linux Server

Issue description

Mentioned on the ENet feature page is the Aggregation feature (http://enet.bespin.org/Features.html) As far as I understand, the way it's supposed to work is that the user puts as much packets as they need, they are cached, and once they call enet_host_service() or enet_host_flush() they are bundled toghether to avoid creating multiple packets for small data. Small packets leads to more overall overhead. This overhead can reach 60-80 bytes (IP + UDP + ENet headers) per packet, which is a waste, if one sends their data in small chunks of 200-500 bytes.

I did a test in Godot 3.5-stable using raw NetworkedMultiplayerENet (without MultiplayerAPI). I assumed ENet would accumulate these small packets (< 300 bytes) for me and send them whenever I call NetworkedMultiplayerPeer.poll() next time.

But upon inspecting UDP packets in Wireshark, I found out that it sends each small packet separately (even though the poll() frequency was intentionally lowered to 10FPS and put_packet() raised to 60FPS)

Upon inspecting the source code, i found this https://github.com/godotengine/godot/blob/338114d47154298cc96bb3098653bb3db3943457/modules/enet/networked_multiplayer_enet.cpp#L614 At the end of NetworkedMultiplayerENet::put_packet(const uint8_t *p_buffer, int p_buffer_size) method, which in my understanding forces the packet to be sent instantly

As such, I am currently forced to manually aggregate packets in some array, make sure its below the MTU and only then call put_packet().

I don't know if that's still present in Godot 4.0 or if something was exposed to support automatic agggregation. It would be nice to know if it was.

Steps to reproduce

Connect two peers via NetworkedMultiplayerENet and send small, but frequent data. Inspect packets using Wireshark or tcpdump to discover they ar enot aggregated and instead sent as-is

Minimal reproduction project

N/A

[Faless]

Well, the flush is there to reduce outgoing latency. I'd rather have 1ms less than consume less bytes. I guess we could make it an option (flush by default), but we should measure the impact on latency (I expect an average 8ms more, given we service once per frame).

On Fri, Jun 30, 2023, 11:48 Hugo Locurcio @.***> wrote:

cc @Faless https://github.com/Faless

— Reply to this email directly, view it on GitHub https://github.com/godotengine/godot/issues/78867#issuecomment-1614414177, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAM4C3WYHDYT3GM65ETSSTDXN2OITANCNFSM6AAAAAAZZSOH4A . You are receiving this because you were mentioned.Message ID: @.***>

[dreadpon]

Maybe enet_host_flush() could be exposed in some way so a manual send could be issued (thus avoiding latency). And automatic flushing (current behavior) could be disabled in the ENet object via some boolean flag.

BTW, ENetConnection from 4.0 seems to have some sort of flush() already. But no flush() in ENetPacketPeer or ENetMultiplayerPeer