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

[Enhancement] Serialize custom packet in networking thread instead of the client/server thread #3335

Closed
qouteall opened this issue Sep 24, 2023 · 2 comments · Fixed by #3407
Labels
enhancement New feature or request fabric-networking Pull requests and issues related to the networking api

Comments

@qouteall
Copy link
Contributor

qouteall commented Sep 24, 2023

Currently, for the new object-based networking API, when creating the packet, it serializes the packet in the clinet/server main thread. Vanilla packets are serialized on networking thread. If the packet is large, moving the serialization to networking thread could improve performance. What's more, in singleplayer, the packet objects are passed by object reference, not serialized or deserialized, to improve performance, so it could also improve performance for singleplayer case.

https://github.com/FabricMC/fabric/blob/1.20.2/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/server/ServerNetworkingImpl.java#L66C21-L66C21

The payload object carries PacketByteBuf. It's possible to make it to carry the packet object instead of buffer. https://github.com/FabricMC/fabric/blob/1.20.2/fabric-networking-api-v1/src/main/java/net/fabricmc/fabric/impl/networking/payload/PacketByteBufPayload.java

This cannot be done to the old networking API because it accepts the PacketByteBuf as argument.

@apple502j
Copy link
Contributor

Currently the new networking API is just a wrapper of the old one. We'll need to change the code there. And yes, @modmuss50 did try this during snapshot cycle, it failed.

@apple502j apple502j added the fabric-networking Pull requests and issues related to the networking api label Sep 24, 2023
@modmuss50
Copy link
Member

it failed.

Not really, it turned out to me more work than I originally expected, so decided to leave it for a later date. Its totally possible to do without breaking the current API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request fabric-networking Pull requests and issues related to the networking api
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants