Performance optimization of chat #3634

Janmm14 · 2024-03-15T14:06:39Z

Add lazy json deserialization of chat content (until BaseComponent is questioned, save only String/Tag).
To achieve this, a Deserializable interface has been added with a few implementations. Existing getters, setters and constructors have been kept as they were, added are new "raw" versions of it, exposing the Deserializable itself.

These performance optimizations are based on the assumption that most plugins and use cases do not involve reading mc-server-sent chat/tablist header/footer/entries inside bungeecord.

Basic testing has been done.

Janmm14 · 2024-03-15T14:18:16Z

I changed a little compared to chat-optimzation-v2.

Better javadoc on Deserializable
Replaced Suppliers with captured original value to Functions which get original value via parameter

andreasdc · 2024-03-15T18:35:41Z

Bukkit servers don't send this in json already? If yes, can't it be used?

Janmm14 · 2024-03-15T18:52:45Z

Bukkit servers don't send this in json already? If yes, can't it be used?

Thats what this pr does. It keeps the json string unless basecomponent requested by other code or a setter used to override.

In newer versions, they do not send json string, but they send nbt. In that case, this PR will omit the conversion of nbt to json, but due to nbt having no size prefix, it needs to read the bytes into nbt.

andreasdc · 2024-03-15T18:59:16Z

Sounds great. 😎 🚀

andreasdc · 2024-03-18T01:08:06Z

java.lang.NullPointerException: Cannot invoke "net.md_5.bungee.protocol.Deserializable.get()" because "this.displayNameRaw" is null
on this line

BungeeCord/protocol/src/main/java/net/md_5/bungee/protocol/packet/PlayerListItem.java

Line 154 in 7d438f6

return displayNameRaw.get();

Maybe I messed something up, I'm not sure. It happens with BungeeTabListPlus.
Maybe it should be like that, I did that and there's no error.

BungeeCord/protocol/src/main/java/net/md_5/bungee/protocol/packet/ScoreboardScore.java

Line 94 in 7d438f6

public BaseComponent getDisplayName()

BTW I guess this patch won't help with BTLP?

Janmm14 · 2024-03-18T12:05:10Z

java.lang.NullPointerException: Cannot invoke "net.md_5.bungee.protocol.Deserializable.get()" because "this.displayNameRaw" is null on this line

BungeeCord/protocol/src/main/java/net/md_5/bungee/protocol/packet/PlayerListItem.java

Line 154 in 7d438f6

return displayNameRaw.get();

Maybe I messed something up, I'm not sure. It happens with BungeeTabListPlus.
Maybe it should be like that, I did that and there's no error.

BungeeCord/protocol/src/main/java/net/md_5/bungee/protocol/packet/ScoreboardScore.java

Line 94 in 7d438f6

public BaseComponent getDisplayName()

BTW I guess this patch won't help with BTLP?

Nice catch, your correction looks correct.

Janmm14 · 2024-03-18T13:01:57Z

Fixed the same thing in other places. And yes, this patch likely does not help performance of bungeetablistplus.

andreasdc · 2024-03-18T13:05:12Z

I'm testing this in production, other that the error that I sent you I don't see any problems. Here are the profilers, I think the performance is better. :)
https://i.imgur.com/fC3RwL1.png

Janmm14 · 2024-03-28T19:49:29Z

Fixed stupid mistake in writeBaseComponenet Janmm14@e275e38
Squashig commits.

Tested basic functionality.

Stop use of subclass for static method call. Make test helper methods static.

md-5 · 2024-03-30T22:40:06Z

Can you please merge/rebase the remaining?

Thanks

Janmm14 · 2024-03-31T00:15:00Z

@md-5 I was able to just cherry-pick the missing main commit 8cd1505 onto master?!

md-5 · 2024-03-31T00:59:31Z

Thanks

md-5 · 2024-04-01T04:47:25Z

protocol/src/main/java/net/md_5/bungee/protocol/Deserializable.java

@@ -0,0 +1,28 @@
+package net.md_5.bungee.protocol;


I think these new classes should go in a separate package (util or otherwise)

md-5 · 2024-04-01T04:48:05Z

protocol/src/main/java/net/md_5/bungee/protocol/packet/BossBar.java

@@ -18,7 +22,7 @@ public class BossBar extends DefinedPacket

    private UUID uuid;
    private int action;
-    private BaseComponent title;
+    private Deserializable<Either<String, SpecificTag>, BaseComponent> title;


I think every instance is "Deserializable<Either<String, SpecificTag>, BaseComponent>", so we should probably create a helper "ChatDeserializable extends Deserializable<Either<String, SpecificTag>, BaseComponent>"

Creating a ChatDeserializable is unfortunaly not working as an alias and would require all implementations to be doubled.
If you want it shorter, the solution I see is to get rid of generics and just start worrying about extendability and code reuse if we actually need it at some later point.

It's a tough one. Maybe leave Deserializable and specialise the implementations?

protocol/src/main/java/net/md_5/bungee/protocol/packet/Team.java

md-5 · 2024-04-01T04:51:07Z

chat/src/main/java/net/md_5/bungee/chat/ComponentSerializer.java

@@ -163,6 +164,16 @@ public static String toString(Object object)
        return gson.toJson( object );
    }

+    public static String toString(Content content)


What's the reason for these being added?

During work on this, I noticed #3629 and wanted to remove any invocations of toString(Object), and here I jsut forgot to mention it.
I would further suggest to deprecate the toString(Object) method here.

Added deprecation

Janmm14 · 2024-04-14T18:12:06Z

~~Fcking import order~~

Janmm14 · 2024-04-18T12:13:41Z

Please do another review.

bob7l · 2024-04-19T06:01:07Z

Hopefully, this gets merged. It will set a precedent for future packet decoding/handling as well. The serialization/deserialization process is likely only going to become more complex/heavy with each new Minecraft version release.

Janmm14 · 2024-04-21T09:36:54Z

Hopefully, this gets merged. It will set a precedent for future packet decoding/handling as well. The serialization/deserialization process is likely only going to become more complex/heavy with each new Minecraft version release.

Just some offtopic chatting:
General packet handling/decoding in bungee already keeps the serialized ByteBuf for decoded packets if they are not changed (changing requires to throw cancelsendsignal in handler and re-send).

Other proposed speedups in packet handling code are:

Don't recompress unchanged packets #3240 keep compressed bytebuf around for use in unchanged packets
#3392: Consolidate flushes, leading to reduces syscalls and higher performance #3393 stop flushing for every packet, only flush after x writes or when the source stopped sending (works best with new minecraft versions or with paper introducing per-tick flushing a few mc versions earlier)

Janmm14 · 2024-04-21T10:10:29Z

Moved unrelated changes I did while working on this PR to their own PRs:

Stop use of subclass for static method call. Make test helper methods static.

andreasdc · 2024-06-19T23:39:32Z

1.21 update? 🚀 Also I don't have any problems with this patch, I think it should be implemented. 🔥

bob7l · 2024-06-20T00:42:16Z

1.21 update? 🚀 Also I don't have any problems with this patch, I think it should be implemented. 🔥

I agree it should be implemented too, but it's going to be very plugin-breaking. A plugin using the BossBar packet for instance, the methods get/set#BaseComponent would no longer exist due to the wrapper replacing the BaseComponent obviously.

So it wouldn't be a quick and painless merge. Will require a large amount of plugins to update.

andreasdc · 2024-06-20T15:21:45Z

1.21 update? 🚀 Also I don't have any problems with this patch, I think it should be implemented. 🔥

I agree it should be implemented too, but it's going to be very plugin-breaking. A plugin using the BossBar packet for instance, the methods get/set#BaseComponent would no longer exist due to the wrapper replacing the BaseComponent obviously.

So it wouldn't be a quick and painless merge. Will require a large amount of plugins to update.

There may be workarounds and plugin authors can update their projects, these classes received updates not so long time ago anyway.

Janmm14 · 2024-06-20T16:40:51Z

1.21 update? 🚀 Also I don't have any problems with this patch, I think it should be implemented. 🔥

I agree it should be implemented too, but it's going to be very plugin-breaking. A plugin using the BossBar packet for instance, the methods get/set#BaseComponent would no longer exist due to the wrapper replacing the BaseComponent obviously.

So it wouldn't be a quick and painless merge. Will require a large amount of plugins to update.

Packets / protocol is not official part of the API.
An earlier version of this had backwards support, but md-5 did not want it.

…st existing code to use new types

…ers and split either-sourced and new-only-sourced.

Janmm14 · 2024-06-20T23:04:33Z

Updated for 1.21 with rebase

Janmm14 mentioned this pull request Mar 15, 2024

Change Gson to faster serialization library #3624

Open

4 tasks

Janmm14 changed the title ~~Optimize chat~~ Performance optimization of chat Mar 15, 2024

Janmm14 force-pushed the optimize-chat branch from bcedc5c to 9d433c5 Compare March 18, 2024 13:00

Janmm14 force-pushed the optimize-chat branch from e275e38 to b979aa6 Compare March 28, 2024 19:49

md-5 pushed a commit that referenced this pull request Mar 30, 2024

#3634: Micro-optimize chat deserialization

c2f73d3

md-5 pushed a commit that referenced this pull request Mar 30, 2024

#3634: Improve chat test code style.

82684c7

Stop use of subclass for static method call. Make test helper methods static.

Janmm14 force-pushed the optimize-chat branch from b979aa6 to d678102 Compare March 31, 2024 00:14

Janmm14 force-pushed the optimize-chat branch from d678102 to 4d36170 Compare March 31, 2024 00:15

md-5 reviewed Apr 1, 2024

View reviewed changes

protocol/src/main/java/net/md_5/bungee/protocol/packet/Team.java Outdated Show resolved Hide resolved

md-5 reviewed Apr 1, 2024

View reviewed changes

Janmm14 force-pushed the optimize-chat branch 4 times, most recently from 6cb2be9 to 0f23c99 Compare April 14, 2024 18:11

Janmm14 force-pushed the optimize-chat branch from 0f23c99 to 6373670 Compare April 21, 2024 10:06

thxrben pushed a commit to thxrben/BungeeCord that referenced this pull request Jun 16, 2024

SpigotMC#3634: Micro-optimize chat deserialization

e97d6ad

thxrben pushed a commit to thxrben/BungeeCord that referenced this pull request Jun 16, 2024

SpigotMC#3634: Improve chat test code style.

0dfd7d8

Stop use of subclass for static method call. Make test helper methods static.

Janmm14 added 6 commits June 21, 2024 00:30

Create util package in protocol

b40f1aa

Create data package in protocol

52432b7

Lazily deserialize BaseComponent in packets

dd45d5b

Update Deserializable implementations to be chat-specific

7cfc2a2

Remove all compatibility accessors/constructors from packets and adju…

c6c9e72

…st existing code to use new types

Change to provide deserializable implementations with inbuilt convert…

21cd215

…ers and split either-sourced and new-only-sourced.

Janmm14 force-pushed the optimize-chat branch from 6373670 to 21cd215 Compare June 20, 2024 23:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Performance optimization of chat #3634

Performance optimization of chat #3634

Janmm14 commented Mar 15, 2024 •

edited

Loading

Janmm14 commented Mar 15, 2024

andreasdc commented Mar 15, 2024 •

edited

Loading

Janmm14 commented Mar 15, 2024 •

edited

Loading

andreasdc commented Mar 15, 2024

andreasdc commented Mar 18, 2024 •

edited

Loading

Janmm14 commented Mar 18, 2024 •

edited

Loading

Janmm14 commented Mar 18, 2024

andreasdc commented Mar 18, 2024

Janmm14 commented Mar 28, 2024 •

edited

Loading

md-5 commented Mar 30, 2024

Janmm14 commented Mar 31, 2024

md-5 commented Mar 31, 2024

md-5 Apr 1, 2024

md-5 Apr 1, 2024

Janmm14 Apr 1, 2024 •

edited

Loading

md-5 Apr 13, 2024

md-5 Apr 1, 2024

Janmm14 Apr 1, 2024

Janmm14 Apr 14, 2024

Janmm14 commented Apr 14, 2024 •

edited

Loading

Janmm14 commented Apr 18, 2024

bob7l commented Apr 19, 2024

Janmm14 commented Apr 21, 2024

Janmm14 commented Apr 21, 2024

andreasdc commented Jun 19, 2024

bob7l commented Jun 20, 2024

andreasdc commented Jun 20, 2024 •

edited

Loading

Janmm14 commented Jun 20, 2024

Janmm14 commented Jun 20, 2024

Performance optimization of chat #3634

Are you sure you want to change the base?

Performance optimization of chat #3634

Conversation

Janmm14 commented Mar 15, 2024 • edited Loading

Janmm14 commented Mar 15, 2024

andreasdc commented Mar 15, 2024 • edited Loading

Janmm14 commented Mar 15, 2024 • edited Loading

andreasdc commented Mar 15, 2024

andreasdc commented Mar 18, 2024 • edited Loading

Janmm14 commented Mar 18, 2024 • edited Loading

Janmm14 commented Mar 18, 2024

andreasdc commented Mar 18, 2024

Janmm14 commented Mar 28, 2024 • edited Loading

md-5 commented Mar 30, 2024

Janmm14 commented Mar 31, 2024

md-5 commented Mar 31, 2024

md-5 Apr 1, 2024

Choose a reason for hiding this comment

md-5 Apr 1, 2024

Choose a reason for hiding this comment

Janmm14 Apr 1, 2024 • edited Loading

Choose a reason for hiding this comment

md-5 Apr 13, 2024

Choose a reason for hiding this comment

md-5 Apr 1, 2024

Choose a reason for hiding this comment

Janmm14 Apr 1, 2024

Choose a reason for hiding this comment

Janmm14 Apr 14, 2024

Choose a reason for hiding this comment

Janmm14 commented Apr 14, 2024 • edited Loading

Janmm14 commented Apr 18, 2024

bob7l commented Apr 19, 2024

Janmm14 commented Apr 21, 2024

Janmm14 commented Apr 21, 2024

andreasdc commented Jun 19, 2024

bob7l commented Jun 20, 2024

andreasdc commented Jun 20, 2024 • edited Loading

Janmm14 commented Jun 20, 2024

Janmm14 commented Jun 20, 2024

Janmm14 commented Mar 15, 2024 •

edited

Loading

andreasdc commented Mar 15, 2024 •

edited

Loading

Janmm14 commented Mar 15, 2024 •

edited

Loading

andreasdc commented Mar 18, 2024 •

edited

Loading

Janmm14 commented Mar 18, 2024 •

edited

Loading

Janmm14 commented Mar 28, 2024 •

edited

Loading

Janmm14 Apr 1, 2024 •

edited

Loading

Janmm14 commented Apr 14, 2024 •

edited

Loading

andreasdc commented Jun 20, 2024 •

edited

Loading