Skip to content

Optimise NetworkManager#515

Closed
AlfieC wants to merge 1 commit intoPaperMC:masterfrom
dedicatedmemers:optimise-networkmanager
Closed

Optimise NetworkManager#515
AlfieC wants to merge 1 commit intoPaperMC:masterfrom
dedicatedmemers:optimise-networkmanager

Conversation

@AlfieC
Copy link
Copy Markdown
Contributor

@AlfieC AlfieC commented Nov 25, 2016

NetworkManager#sendPacket is an extremely hot piece of code - and in one of the methods called, m(), ConcurrentLinkedQueue#isEmpty is called countlessly. We can cut down on these isEmpty() calls by simply checking if poll() is null.

This is pretty minor, but with the frequency in which sendPacket is called, will add up.

@aikar
Copy link
Copy Markdown
Member

aikar commented Nov 25, 2016

Please redo by simply changing it so its like

NetworkManager.QueuedPacket networkmanager_queuedpacket;
while((networkmanager_queuedpacket = this.i.poll()) != null) {

For this, we don't want to comment out, we want to change the lines to help ensure it conflicts on an update. (Conflicts are good for this)

@AlfieC AlfieC force-pushed the optimise-networkmanager branch from ca96fce to fd93597 Compare November 25, 2016 20:21
@AlfieC AlfieC force-pushed the optimise-networkmanager branch from fd93597 to 62e4180 Compare November 25, 2016 20:35
@AlfieC
Copy link
Copy Markdown
Contributor Author

AlfieC commented Nov 25, 2016

Changed PR to completely eliminate the un-needed packet queue.

@Black-Hole
Copy link
Copy Markdown
Contributor

This might conflict with #432 @stonar96

@AlfieC
Copy link
Copy Markdown
Contributor Author

AlfieC commented Nov 26, 2016

@Black-Hole this shouldn't conflict because the queue in @stonar96's patch is unused anyway - no idea why it was there to begin with

@Techcable
Copy link
Copy Markdown
Contributor

It's there to queue packets sent before the player completely connects.
Minecraft is free to call sendPacket() before netty's channel is open and the player's connected.
Instead of removing it completely, maybe you can just set it to null after the player's connected so we don't break anything.

@AlfieC
Copy link
Copy Markdown
Contributor Author

AlfieC commented Nov 26, 2016

After throwing debug checks in all versions, 1.7 -> 1.11, this packet queue has never been used to successfully send a packet.

@zachbr
Copy link
Copy Markdown
Contributor

zachbr commented Nov 27, 2016

Merged with 324acd6

@zachbr zachbr closed this Nov 27, 2016
@Black-Hole
Copy link
Copy Markdown
Contributor

Applying the Anti-Xray-Patch fails very hard here:
https://github.com/PaperMC/Paper/pull/432/files#diff-c15db7ccbd57c4989353f7124f7b6039R1310

@aikar
Copy link
Copy Markdown
Member

aikar commented Nov 28, 2016

@Black-Hole that part can just be removed. As we can see, that change was apparently unneeded to begin with.

@aikar aikar mentioned this pull request Sep 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants