Skip to content

Conversation

@JNNGL
Copy link
Contributor

@JNNGL JNNGL commented Sep 7, 2023

This passes compressed packets that are not present in the packet registry to the backend server/client without decompressing/recompressing them whenever possible; only the first 5 bytes are decompressed to get the packet id.

@JNNGL JNNGL changed the title Pass compressed packets directly to the backend server Pass compressed packets directly to the backend server/client Sep 26, 2023
@astei
Copy link
Contributor

astei commented Oct 27, 2023

This has been proposed many times. By all means, this is not a bad idea. My main concern is that it could introduce overhead from decompressing to check the packet ID. zlib decompression is actually really cheap, the actual CPU-intensive part is compressing the packet. Personally, I would rather have Mojang pull out the packet ID from compressed packets, but the window to do that was in 2013 and that's well past us now.

JNNGL added 3 commits October 30, 2023 23:01
…d-packets-to-backend

# Conflicts:
#	proxy/src/main/java/com/velocitypowered/proxy/connection/MinecraftConnection.java
#	proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftCompressAndIdDecoder.java
#	proxy/src/main/java/com/velocitypowered/proxy/server/VelocityRegisteredServer.java
@JNNGL
Copy link
Contributor Author

JNNGL commented Oct 30, 2023

My main concern is that it could introduce overhead from decompressing to check the packet ID.

Wouldn't that still be faster than recompressing every packet?

@astei
Copy link
Contributor

astei commented Oct 30, 2023

Wouldn't that still be faster than recompressing every packet?

Compressing small packets is still pretty cheap.

Could we have a carveout for packets with a compressed size lower than, say, 2048 bytes? Larger packets will benefit from this much more than smaller packets.

@ytnoos
Copy link

ytnoos commented Oct 30, 2023

Maybe make a configurable amount

@4drian3d 4drian3d added the type: feature New feature or request label Jan 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants