ProtocolLib

3M Downloads

Performance issues with protocollib flushing every packet

MWHunter opened this issue ยท 12 comments

commented

Describe the bug
There are performance issues with protocollib. It seems to be flushing every packet instead of letting them queue. This is very inefficient and contributes to it using the most netty resources out of any plugin, even when no plugins are actually using protocollib.

To Reproduce
Steps to reproduce the behavior:

  1. Profile a server with spark --thread *, any version from 1.8 to 1.17, and you will see increased netty usage over previous versions.

Expected behavior
ProtocolLib should not be using the most netty usage out of any plugin

Screenshots
This server has no plugins using ProtocolLib - if you go to the plugins tab, protocollib is taking a large amount of the netty thread despite this.
With protocollib: https://spark.lucko.me/F1VwD1us3W
Without protocollib: https://spark.lucko.me/vLYzWP7PxB

Version Info
Version 4.7.0

Additional context
This seems to affect all servers using ProtocolLib.

commented

Any news about this ?

commented

As the given spark links are expired, and any recent spark report on this issue tracker was not really helpful / didn't contain any information which pointed to the same issue provided here. Also I don't really see what you mean by "letting them queue", minecraft itself writes and flushes each packet, thats not the fault of protocol lib...

commented

This is wrong. Paper is intelligent about when it flushes entity tracking papers. While vanilla is inefficient, ProtocolLib undoes Paper optimizations. Additionally, other plugins are writing without flushing, and forcing the flush will slow the other plugins down.

commented

Would love to see the timings report there, I don't see any explicit calls to writeAndFlush or flush anywhere on the first look...

commented

On this flare I got a lot of flushing packets
https://flare.airplane.gg/d05507db

image
image

commented

I will take a look in the next days if I can reproduce this, and (if i can reporoduce) dive into it and look for a fix. However on the first look on the screenshots it just looks like a packet write which gets called to a lot of handlers in the pipeline... Will see

commented

While you are at it, can you also prevent ProtocolLib from immediately calling the vanilla encoder via reflection after they process an outgoing packet? I don't understand why they do it, when it will be done anyway as its passed in the pipeline. Just makes other projects unable to process NMS packets ProtocolLib modified, and it just makes no sense to me. Thanks.

commented

Actually already made an issue regarding that #1522

commented

So, this is completely fixed ?

commented

Should be, didn't notice any reports in a while

commented

There's still the performance impact of unnecessary sync listeners instead of async, but that's on plugins with 5.0 not ProtocolLib.

commented

Then it's perfect, I think it can be closed