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

add back optimise non-flush packet sending patch #10172

Conversation

wuangg
Copy link

@wuangg wuangg commented Jan 17, 2024

There are reasons I can think of why the original patch was dropped:

  1. Mojang made some code changes in order to optimize the networking performance in 1.20.2, as some of these changes were similar to a collection of patches had already existed in Paper, resulting in the removal of these patches.
  2. AbstractEventExecutor.LazyRunnable had been deprecated since the release of Netty 4.1.92 (see Deprecates LazyRunnable netty/netty#13335)

Although there is an alternative mechanism provided from the API reference of AbstractEventExecutor, the solution in this PR only requires a small amount of code changes.

@wuangg wuangg requested a review from a team as a code owner January 17, 2024 12:24
@wuangg wuangg force-pushed the feature/optimise-non-flush-packet-sending branch from 178960d to fea1f08 Compare January 17, 2024 14:19
@Potothingi
Copy link

Has this been tested? I can't connect to the server.

@electronicboy
Copy link
Member

it's a small change in which packets which need to get out fast are generally marked as such, and this is working fine here, however, I do test behind a bungee proxy

@wuangg wuangg force-pushed the feature/optimise-non-flush-packet-sending branch from fea1f08 to d29296a Compare January 17, 2024 18:58
@wuangg
Copy link
Author

wuangg commented Jan 17, 2024

Has this been tested? I can't connect to the server.

Oops.

Yes this has been tested and indeed was working, later I was testing out @electronicboy 's suggestion (which causes the player cannot connect to the server) and accidentally pushed that code change as I was out of focus for a bit.

The change has been reverted, now this PR should work fine just like at the beginning and I have to be careful next time. Thank you for hitting me up!

@Potothingi
Copy link

This patch broke the plugin that reads and modifies chat packets with ProtocolLib.

@wuangg
Copy link
Author

wuangg commented Jan 19, 2024

This patch broke the plugin that reads and modifies chat packets with ProtocolLib.

Are there any log about this?

@wuangg wuangg marked this pull request as draft January 19, 2024 06:57
@electronicboy
Copy link
Member

@wuangg
Copy link
Author

wuangg commented Jan 19, 2024

It seems like I have to open a PR on ProtocolLib to make their event loop hook extending AbstractEventLoop. I still have another alternative approach but it would be too much in terms of code diffs

@wuangg
Copy link
Author

wuangg commented Feb 9, 2024

Close this until I came up with a suitable solution.

@wuangg wuangg closed this Feb 9, 2024
@wuangg wuangg deleted the feature/optimise-non-flush-packet-sending branch February 9, 2024 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Closed
Development

Successfully merging this pull request may close these issues.

3 participants