search

PaperMC / Waterfall

BungeeCord fork that aims to improve performance and stability.
https://papermc.io
MIT License
742 stars 297 forks source link

Use ByteBuf#copy only when entity rewrite is actually used #799

Open BoomEaro opened 1 year ago

BoomEaro commented 1 year ago

This pull request returns the ability to slice ByteBuf when decoding a packet in the MinecraftDecoder class as it did before, but only if entity rewrite was never used. If I understand correctly, the original reason for using copy is that entity rewrite can write more bytes than is possible in the slice itself. Therefore, most likely there is no point in constantly copying bytes even when we are not going to change them.

I would like to know your opinion on this, what it could possibly break and whether it makes any sense at all, since I have not had much experience with netty yet.

electronicboy commented 1 year ago

This was long on the list of stuff that I wanted to look into, given that we're not rewriting this stuff, I did wanna look into adding a mechanism to literally just skip packets and let the proxy not care about them, even if registered

Janmm14 commented 1 year ago

There's no such thing as skipping packets, you always have to check packet id. And slicing instead of copying is already a big progress. Not sure what else can be done here to optimize performance.

electronicboy commented 1 year ago

I mean skip the decode + handle + encode chain for unneeded packets in certain configs; if we're not rewriting entity metadata, then we can skip a bunch of work

Janmm14 commented 1 year ago

I mean skip the decode + handle + encode chain for unneeded packets in certain configs; if we're not rewriting entity metadata, then we can skip a bunch of work

Bungee doesn't use decoding into object fields for most entity rewriting stuff.

electronicboy commented 1 year ago

True, but part of it was for other stuff like scoreboards, etc; it's probably not a big deal, and probs not something I'm ever going to care to get to considering the state of this project

bob7l commented 1 year ago

This commit would likely break several plugins like ViaVersion that are expecting a grow-able, copied buffer. If anything, it should be made into a configurable option for the user to decide.

Janmm14 commented 1 year ago

This commit would likely break several plugins like ViaVersion that are expecting a grow-able, copied buffer. If anything, it should be made into a configurable option for the user to decide.

Statement from someone knowing viaverson-bungee is needed. Don't want some useless config option or startup flag.

xism4 commented 1 year ago

This would only theoretically occur if the option were disabled