From 3e549c49034f071e73cb747600918e1a93ba41c9 Mon Sep 17 00:00:00 2001 From: MrPowerGamerBR Date: Sat, 18 Nov 2023 21:32:54 -0300 Subject: [PATCH] New optimization patches --- README.md | 8 +++ ...imize-ServerStatsCounter-s-dirty-set.patch | 57 +++++++++++++++++++ ...-unnecessary-ItemFrame-getItem-calls.patch | 55 ++++++++++++++++++ ...Track-how-much-MSPT-each-world-used.patch} | 0 ...atch => 0011-Parallel-world-ticking.patch} | 0 5 files changed, 120 insertions(+) create mode 100644 patches/server/0008-Optimize-ServerStatsCounter-s-dirty-set.patch create mode 100644 patches/server/0009-Avoid-unnecessary-ItemFrame-getItem-calls.patch rename patches/server/{0008-Track-how-much-MSPT-each-world-used.patch => 0010-Track-how-much-MSPT-each-world-used.patch} (100%) rename patches/server/{0009-Parallel-world-ticking.patch => 0011-Parallel-world-ticking.patch} (100%) diff --git a/README.md b/README.md index 8062c02..df437f1 100644 --- a/README.md +++ b/README.md @@ -26,6 +26,14 @@ SparklyPaper's config file is `sparklypaper.yml`, the file is, by default, place * But still, if you made your own custom "image on map" plugin, don't forget to `mapView.isLocked = true` to get the same performance benefits in vanilla Paper! * Fix concurrency issues when using `imageToBytes` in multiple threads * Useful if one of your plugins is parallelizng map creation on server startup +* Optimize `ServerStatsCounter`'s dirty set + * Instead of using Java's HashSet, we will use fastutil's `ObjectOpenHashSet`, which has better performance + * While this seems stupid, awardStat was using around ~0.14% when adding to the `HashSet`, and that's not good + * We also optimized the `getDirty` calls. I mean, the *only* `getDirty` call. Because the map was only retrieved once, we don't actually need to create a copy of the map just to iterate it, we can just access it directly and clear it manually after use. +* Avoid unnecessary `ItemFrame#getItem()` calls + * When ticking an item frame, on each tick, it checks if the item on the item frame is a map and, if it is, it adds the map to be carried by the entity player + * However, the `getItem()` call is a bit expensive, especially because this is only really used if the item in the item frame is a map + * We can avoid this call by checking if the `cachedMapId` is not null, if it is, then we get the item in the item frame, if not, then we ignore the `getItem()` call. * Check how much MSPT (milliseconds per tick) each world is using in `/mspt` * Useful to figure out which worlds are lagging your server. ![Per World MSPT](docs/per-world-mspt.png) diff --git a/patches/server/0008-Optimize-ServerStatsCounter-s-dirty-set.patch b/patches/server/0008-Optimize-ServerStatsCounter-s-dirty-set.patch new file mode 100644 index 0000000..fea1fb9 --- /dev/null +++ b/patches/server/0008-Optimize-ServerStatsCounter-s-dirty-set.patch @@ -0,0 +1,57 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: MrPowerGamerBR +Date: Sat, 18 Nov 2023 20:35:44 -0300 +Subject: [PATCH] Optimize ServerStatsCounter's dirty set + +Instead of using Java's HashSet, we will use fastutil's ObjectOpenHashSet, which has better performance + +While this seems stupid, awardStat was using around ~0.14% when adding to the HashSet, and that's not good + +We also optimized the getDirty calls, since the map was only retrieved once, so in that case, we don't actually need to create a copy of the map just to iterate it + +diff --git a/src/main/java/net/minecraft/stats/ServerStatsCounter.java b/src/main/java/net/minecraft/stats/ServerStatsCounter.java +index 9bb8d4d7be6a937980aa653db82be084d066a563..f2c1a0046075ea0f3a06fdbe40ea4b776203b37c 100644 +--- a/src/main/java/net/minecraft/stats/ServerStatsCounter.java ++++ b/src/main/java/net/minecraft/stats/ServerStatsCounter.java +@@ -43,7 +43,7 @@ public class ServerStatsCounter extends StatsCounter { + private static final Logger LOGGER = LogUtils.getLogger(); + private final MinecraftServer server; + private final File file; +- private final Set> dirty = Sets.newHashSet(); ++ private final Set> dirty = new it.unimi.dsi.fastutil.objects.ObjectOpenHashSet(); // SparklyPaper - optimize ServerStatsCounter's dirty set + + public ServerStatsCounter(MinecraftServer server, File file) { + this.server = server; +@@ -85,12 +85,15 @@ public class ServerStatsCounter extends StatsCounter { + this.dirty.add(stat); + } + ++ // SparklyPaper - Optimize ServerStatsCounter's dirty set ++ /* + private Set> getDirty() { +- Set> set = Sets.newHashSet(this.dirty); ++ Set> set = new it.unimi.dsi.fastutil.objects.ObjectOpenHashSet<>(this.dirty); // SparklyPaper - optimize ServerStatsCounter's dirty set + + this.dirty.clear(); + return set; + } ++ */ + + public void parseLocal(DataFixer dataFixer, String json) { + try { +@@ -238,13 +241,14 @@ public class ServerStatsCounter extends StatsCounter { + + public void sendStats(ServerPlayer player) { + Object2IntMap> object2intmap = new Object2IntOpenHashMap(); +- Iterator iterator = this.getDirty().iterator(); ++ Iterator iterator = this.dirty.iterator(); // SparklyPaper - Optimize ServerStatsCounter's dirty set + + while (iterator.hasNext()) { + Stat statistic = (Stat) iterator.next(); + + object2intmap.put(statistic, this.getValue(statistic)); + } ++ this.dirty.clear(); // SparklyPaper - Optimize ServerStatsCounter's dirty set + + player.connection.send(new ClientboundAwardStatsPacket(object2intmap)); + } diff --git a/patches/server/0009-Avoid-unnecessary-ItemFrame-getItem-calls.patch b/patches/server/0009-Avoid-unnecessary-ItemFrame-getItem-calls.patch new file mode 100644 index 0000000..c6a34f0 --- /dev/null +++ b/patches/server/0009-Avoid-unnecessary-ItemFrame-getItem-calls.patch @@ -0,0 +1,55 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: MrPowerGamerBR +Date: Sat, 18 Nov 2023 21:04:53 -0300 +Subject: [PATCH] Avoid unnecessary ItemFrame#getItem() calls + +When ticking a item frame, on each tick, it checks if the item on the item frame is a map and, if it is, it adds the map to be carried by the entity player + +However, the "getItem()" call is a bit expensive, especially because this is only really used if the item in the item frame is a map + +We can avoid this call by checking if the "cachedMapId" is not null + +diff --git a/src/main/java/net/minecraft/server/level/ServerEntity.java b/src/main/java/net/minecraft/server/level/ServerEntity.java +index aedf24ba0d64de855a59869052cbc2704e7dc134..f49585cfd485eed504e91887599ff25c3238c8fd 100644 +--- a/src/main/java/net/minecraft/server/level/ServerEntity.java ++++ b/src/main/java/net/minecraft/server/level/ServerEntity.java +@@ -115,13 +115,14 @@ public class ServerEntity { + ItemFrame entityitemframe = (ItemFrame) entity; + + if (true || this.tickCount % 10 == 0) { // CraftBukkit - Moved below, should always enter this block +- ItemStack itemstack = entityitemframe.getItem(); +- +- if (this.level.paperConfig().maps.itemFrameCursorUpdateInterval > 0 && this.tickCount % this.level.paperConfig().maps.itemFrameCursorUpdateInterval == 0 && itemstack.getItem() instanceof MapItem) { // CraftBukkit - Moved this.tickCounter % 10 logic here so item frames do not enter the other blocks // Paper - Make item frame map cursor update interval configurable ++ if (this.level.paperConfig().maps.itemFrameCursorUpdateInterval > 0 && this.tickCount % this.level.paperConfig().maps.itemFrameCursorUpdateInterval == 0 && entityitemframe.cachedMapId != null) { // CraftBukkit - Moved this.tickCounter % 10 logic here so item frames do not enter the other blocks // Paper - Make item frame map cursor update interval configurable // SparklyPaper - avoid unnecessary ItemFrame#getItem() calls + Integer integer = entityitemframe.cachedMapId; // Paper + MapItemSavedData worldmap = MapItem.getSavedData(integer, this.level); + + if (worldmap != null) { ++ // SparklyPaper start - avoid unnecessary ItemFrame#getItem() calls ++ ItemStack itemstack = entityitemframe.getItem(); ++ if (itemstack.getItem() instanceof MapItem) { // fail-safe, what if the cached map ID is present but the item isn't a MapItem? (this should NEVER happen but, who knows) + Iterator iterator = this.trackedPlayers.iterator(); // CraftBukkit + + while (iterator.hasNext()) { +@@ -134,6 +135,7 @@ public class ServerEntity { + entityplayer.connection.send(packet); + } + } ++ } // SparklyPaper end + } + } + +diff --git a/src/main/java/net/minecraft/world/entity/decoration/ItemFrame.java b/src/main/java/net/minecraft/world/entity/decoration/ItemFrame.java +index 759ecd79534a7706f7d4a63eb9dacbefcfe54674..07739c3d74074b2668466250f944dfbe22d4dc86 100644 +--- a/src/main/java/net/minecraft/world/entity/decoration/ItemFrame.java ++++ b/src/main/java/net/minecraft/world/entity/decoration/ItemFrame.java +@@ -50,7 +50,8 @@ public class ItemFrame extends HangingEntity { + public static final int NUM_ROTATIONS = 8; + public float dropChance; + public boolean fixed; +- public Integer cachedMapId; // Paper ++ @Nullable // SparklyPaper - avoid unnecessary ItemFrame#getItem() calls ++ public Integer cachedMapId = null; // Paper // SparklyPaper - avoid unnecessary ItemFrame#getItem() calls + + public ItemFrame(EntityType type, Level world) { + super(type, world); diff --git a/patches/server/0008-Track-how-much-MSPT-each-world-used.patch b/patches/server/0010-Track-how-much-MSPT-each-world-used.patch similarity index 100% rename from patches/server/0008-Track-how-much-MSPT-each-world-used.patch rename to patches/server/0010-Track-how-much-MSPT-each-world-used.patch diff --git a/patches/server/0009-Parallel-world-ticking.patch b/patches/server/0011-Parallel-world-ticking.patch similarity index 100% rename from patches/server/0009-Parallel-world-ticking.patch rename to patches/server/0011-Parallel-world-ticking.patch