use composite buffers where possible

[Outfluencer]

i think this should reduce memory allocations I have tested it, its running without any issues for me. But i did no perfromace tests for now, so they are welcome.

[Janmm14]

This should crash for native encryption for packets with size smaller than encryption threshold, as native encryption also requires a singular memory address which a composite bytebuf cannot deliver.

[Outfluencer]

oh you're probably right i tested it on an arm server (custom natives are not supported)

[Janmm14]

The naming of fast boolean in my opinion should be something like canUseComposite. And your complete removal from the length field prepender creates simplicity, but I think we could have it on fast and then if the CipherEncoder is added, we could set the prepender to's canUseComposite to false if we use natives, same for when adding compress. https://github.com/SpigotMC/BungeeCord/blob/84ac7ab944dc911c333b4ef7d16e185c4c2625b7/proxy/src/main/java/net/md_5/bungee/connection/InitialHandler.java#L510 https://github.com/SpigotMC/BungeeCord/blob/master/proxy/src/main/java/net/md_5/bungee/netty/ChannelWrapper.java#L172

(i would just leave out the logic to re-enable canUseComposite on compress removal if cipher not present tho)

[Outfluencer]

The naming of fast boolean in my opinion should be something like canUseComposite. And your complete removal from the length field prepender creates simplicity, but I think we could have it on fast and then if the CipherEncoder is added, we could set the prepender to's canUseComposite to false if we use natives, same for when adding compress.

https://github.com/SpigotMC/BungeeCord/blob/84ac7ab944dc911c333b4ef7d16e185c4c2625b7/proxy/src/main/java/net/md_5/bungee/connection/InitialHandler.java#L510

https://github.com/SpigotMC/BungeeCord/blob/master/proxy/src/main/java/net/md_5/bungee/netty/ChannelWrapper.java#L172 (i would just leave out the logic to re-enable canUseComposite on compress removal if cipher not present tho)

sounds logic and simple, i will change it.

[Outfluencer]

(i would just leave out the logic to re-enable canUseComposite on compress removal if cipher not present tho)

why isn't it simple to just set useComposite to true again?

[Outfluencer]

The naming of fast boolean in my opinion should be something like canUseComposite. And your complete removal from the length field prepender creates simplicity, but I think we could have it on fast and then if the CipherEncoder is added, we could set the prepender to's canUseComposite to false if we use natives, same for when adding compress.

The prepender is an instance (its shared), should we create a new one per connection, or should we add another class and switch them in the pipeline. I prefer per channel Varint21LengthFieldPrepender

[Janmm14]

The naming of fast boolean in my opinion should be something like canUseComposite. And your complete removal from the length field prepender creates simplicity, but I think we could have it on fast and then if the CipherEncoder is added, we could set the prepender to's canUseComposite to false if we use natives, same for when adding compress.

The prepender is an instance (its shared), should we create a new one per connection, or should we add another class and switch them in the pipeline. I prefer per channel Varint21LengthFieldPrepender

Yeah I would un-share it (change to per-connection) instead of replacing the impl.

(i would just leave out the logic to re-enable canUseComposite on compress removal if cipher not present tho)

why isn't it simple to just set useComposite to true again?

I thought that, as the minecraft protocol does not support this and when it was supported it could cause problems due to missing acknowledgement packet (as the threshold applies both ways), we could just not care about this potential performance penalty when compression is removed.

[Outfluencer]

I don't think that it could break anything. As there are no raceconditions because evey operation is performed on the same thread right? (its not possible that somehow this boolean is changed while a packet write or read in the pipeline is processed) It can only change the encoder behaviour of the next packet sent

But i will test what happens if i change the compression multiple times

[Janmm14]

The naming of fast boolean in my opinion should be something like canUseComposite. And your complete removal from the length field prepender creates simplicity, but I think we could have it on fast and then if the CipherEncoder is added, we could set the prepender to's canUseComposite to false if we use natives, same for when adding compress.

The prepender is an instance (its shared), should we create a new one per connection, or should we add another class and switch them in the pipeline. I prefer per channel Varint21LengthFieldPrepender

Oh yeah then the prepender should be per-channel.

(i would just leave out the logic to re-enable canUseComposite on compress removal if cipher not present tho)

why isn't it simple to just set useComposite to true again?

Well its possible, doesn't care, but as its a code path that's essentially unused

[Janmm14]

purely codewise looks good to me now!

[andrewkm]

@md-5 this commit is causing all sorts of issues with various plugins, as has been reported to me by a few other server owners, not to mention it broke floodgate as well, thus I can't even properly test myself.

[md-5]

If it's a plugin not using the API then it's not really a bungee issue. Given that networking isn't exposed to plugins this is almost certainly the case.

[Outfluencer]

@md-5 this commit is causing all sorts of issues with various plugins, as has been reported to me by a few other server owners, not to mention it broke floodgate as well, thus I can't even properly test myself.

Floodgate accessed a private static final field that does not exist anymore.

[Outfluencer]

@andrewkm i fixed the floodgate issue

[andrewkm]

@andrewkm i fixed the floodgate issue

Thank you! :)