diff --git a/README.md b/README.md index b88f531..0c45d41 100644 --- a/README.md +++ b/README.md @@ -60,6 +60,11 @@ SparklyPaper's config file is `sparklypaper.yml`, the file is, by default, place * The `getChunkKey(...)` call is a bit expensive, using 0.24% of CPU time with 19k chunks loaded. * So instead of paying the price on each tick, we pay the price when the chunk is loaded. * Which, if you think about it, is actually better, since we tick chunks more than we load chunks. +* Optimize `canSee(...)` checks + * The `canSee(...)` checks is in a hot path (`ChunkMap#updatePlayers()`), invoked by each entity for each player on the server if they are in tracking range, so optimizing it is pretty nice. + * First, we change the original `HashMap` to fastutil's `Object2ObjectOpenHashMap`, because fastutil's `containsKey` throughput is better. + * Then, we add a `isEmpty()` check before attempting to check if the map contains something. This seems stupid, but it does seem that it improves the performance a bit, and it makes sense, `containsKey(...)` does not attempt to check the map size before attempting to check if the map contains the key. + * We also create a `canSee` method tailored for `ChunkMap#updatePlayer()`, a method without the equals check (the `updatePlayer()` already checks if the entity is the same entity) and we cache the `isVisibleByDefault()` result between runs (this also seems a bit overkill because `isVisibleByDefault()` is just a method that returns a boolean, but because this is a hot path, we need all optimizations that we can get!Draem). * 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/0016-Optimize-canSee-checks.patch b/patches/server/0016-Optimize-canSee-checks.patch new file mode 100644 index 0000000..c8f0c9a --- /dev/null +++ b/patches/server/0016-Optimize-canSee-checks.patch @@ -0,0 +1,75 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: MrPowerGamerBR +Date: Thu, 23 Nov 2023 14:36:47 -0300 +Subject: [PATCH] Optimize "canSee" checks + +The "canSee" checks is in a hot path, invoked by each entity for each player on the server if they are in tracking range, so optimizing it is pretty nice + +First, we change the original "HashMap" to fastutil's "Object2ObjectOpenHashMap", because the containsKey throughput is better + +Then, we add a "isEmpty()" check before attempting to check if the map contains something + +This seems stupid, but it does seem that it improves the performance a bit, and it makes sense, "containsKey(...)" does not attempt to check the map size before attempting to check if the map contains the key + +We also create a "canSee" method tailored for "ChunkMap#updatePlayer()", a method without the equals check (the "updatePlayer()" already checks if the entity is the same entity) and we cache the "isVisibleByDefault()" result between runs (this also seems a bit overkill because "isVisibleByDefault()" is just a method that returns a boolean, but because this is a hotpath, we need all optimizations that we can get + +diff --git a/src/main/java/net/minecraft/server/level/ChunkMap.java b/src/main/java/net/minecraft/server/level/ChunkMap.java +index caa73632aee15583c6b6ed12a668c8f49b794708..6fff96c1c0fc1442803718fa36ac30a35b6ae2d7 100644 +--- a/src/main/java/net/minecraft/server/level/ChunkMap.java ++++ b/src/main/java/net/minecraft/server/level/ChunkMap.java +@@ -1326,6 +1326,7 @@ public class ChunkMap extends ChunkStorage implements ChunkHolder.PlayerProvider + private final int range; + SectionPos lastSectionPos; + public final Set seenBy = new it.unimi.dsi.fastutil.objects.ReferenceOpenHashSet<>(); // Paper - optimise map impl ++ boolean visibleByDefault; // SparklyPaper - optimize canSee checks + + public TrackedEntity(Entity entity, int i, int j, boolean flag) { + this.serverEntity = new ServerEntity(ChunkMap.this.level, entity, j, flag, this::broadcast, this.seenBy); // CraftBukkit +@@ -1340,6 +1341,7 @@ public class ChunkMap extends ChunkStorage implements ChunkHolder.PlayerProvider + final void updatePlayers(com.destroystokyo.paper.util.misc.PooledLinkedHashSets.PooledObjectLinkedOpenHashSet newTrackerCandidates) { + com.destroystokyo.paper.util.misc.PooledLinkedHashSets.PooledObjectLinkedOpenHashSet oldTrackerCandidates = this.lastTrackerCandidates; + this.lastTrackerCandidates = newTrackerCandidates; ++ this.visibleByDefault = entity.visibleByDefault; // SparklyPaper - optimize canSee checks + + if (newTrackerCandidates != null) { + Object[] rawData = newTrackerCandidates.getBackingSet(); +@@ -1436,7 +1438,7 @@ public class ChunkMap extends ChunkStorage implements ChunkHolder.PlayerProvider + // Paper end - check Y + + // CraftBukkit start - respect vanish API +- if (flag && !player.getBukkitEntity().canSee(this.entity.getBukkitEntity())) { // Paper - only consider hits ++ if (flag && !player.getBukkitEntity().canSeeChunkMapUpdatePlayer(this.entity.getBukkitEntity(), visibleByDefault)) { // Paper - only consider hits // SparklyPaper - optimize canSee checks + flag = false; + } + // CraftBukkit end +diff --git a/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java b/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java +index 83aaf3e6e377d731ce02f779f80b7bf5db46f89f..3a02f92b57563b42680f4b867d681d2232c748cc 100644 +--- a/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java ++++ b/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java +@@ -183,7 +183,7 @@ public class CraftPlayer extends CraftHumanEntity implements Player { + private boolean hasPlayedBefore = false; + private final ConversationTracker conversationTracker = new ConversationTracker(); + private final Set channels = new HashSet(); +- private final Map>> invertedVisibilityEntities = new HashMap<>(); ++ private final Map>> invertedVisibilityEntities = new it.unimi.dsi.fastutil.objects.Object2ObjectOpenHashMap<>(); // SparklyPaper - optimize canSee checks + private final Set unlistedEntities = new HashSet<>(); // Paper + private static final WeakHashMap> pluginWeakReferences = new WeakHashMap<>(); + private int hash = 0; +@@ -2070,9 +2070,16 @@ public class CraftPlayer extends CraftHumanEntity implements Player { + + @Override + public boolean canSee(org.bukkit.entity.Entity entity) { +- return this.equals(entity) || entity.isVisibleByDefault() ^ this.invertedVisibilityEntities.containsKey(entity.getUniqueId()); // SPIGOT-7312: Can always see self ++ return this.equals(entity) || entity.isVisibleByDefault() ^ (!invertedVisibilityEntities.isEmpty() && this.invertedVisibilityEntities.containsKey(entity.getUniqueId())); // SPIGOT-7312: Can always see self // SparklyPaper - optimize canSee checks + } + ++ // SparklyPaper - optimize canSee checks ++ // The check in ChunkMap#updatePlayer already rejects if it is the same entity, so we don't need to check it twice, especially because CraftPlayer's equals check is a bit expensive ++ public boolean canSeeChunkMapUpdatePlayer(org.bukkit.entity.Entity entity, boolean visibleByDefault) { ++ return visibleByDefault ^ (!invertedVisibilityEntities.isEmpty() && this.invertedVisibilityEntities.containsKey(entity.getUniqueId())); // SPIGOT-7312: Can always see self // SparklyPaper - optimize canSee checks ++ } ++ // SparklyPaper end ++ + public boolean canSee(UUID uuid) { + org.bukkit.entity.Entity entity = this.getServer().getPlayer(uuid); + if (entity == null) { diff --git a/patches/server/0016-Track-how-much-MSPT-each-world-used.patch b/patches/server/0017-Track-how-much-MSPT-each-world-used.patch similarity index 100% rename from patches/server/0016-Track-how-much-MSPT-each-world-used.patch rename to patches/server/0017-Track-how-much-MSPT-each-world-used.patch diff --git a/patches/server/0017-Parallel-world-ticking.patch b/patches/server/0018-Parallel-world-ticking.patch similarity index 100% rename from patches/server/0017-Parallel-world-ticking.patch rename to patches/server/0018-Parallel-world-ticking.patch