9
0
mirror of https://github.com/SparklyPower/SparklyPaper.git synced 2025-12-19 15:09:27 +00:00

Remove some "dubious" patches

Honestly, some of these changes felt like hopium "surely this will fix the lag issues", even tho in my mind these didn't make sense

Yeah, these methods do show up in the profiler, but does these *really* make sense? I don't think so, I think these are just useless patches that only attempt to hide the pain, instead of tackling the REAL issue that is causing these methods to show up in the profiler
This commit is contained in:
MrPowerGamerBR
2023-11-24 02:48:58 -03:00
parent e509994f65
commit d17719e4e3
7 changed files with 11 additions and 86 deletions

View File

@@ -53,9 +53,6 @@ SparklyPaper's config file is `sparklypaper.yml`, the file is, by default, place
* The quintessential patch that other performance forks also have for... some reason??? I thought that this optimization was too funny to not do it in SparklyPaper. * The quintessential patch that other performance forks also have for... some reason??? I thought that this optimization was too funny to not do it in SparklyPaper.
* Caches when Bat's spooky season starts and ends, and when Skeleton and Zombies halloween starts and ends. The epoch is updated every 90 days. If your server is running for 90+ days straight without restarts, congratulations! * Caches when Bat's spooky season starts and ends, and when Skeleton and Zombies halloween starts and ends. The epoch is updated every 90 days. If your server is running for 90+ days straight without restarts, congratulations!
* Avoids unnecessary date checks, even tho that this shouldn't really improve performance that much... unless you have a lot of bats/zombies/skeletons spawning. * Avoids unnecessary date checks, even tho that this shouldn't really improve performance that much... unless you have a lot of bats/zombies/skeletons spawning.
* Cache tracking range type enum ordinal
* Yes, I was shocked, flabbergasted even, when I found out that `Enum.ordinal()` was lagging when calling `Entity.getPlayersInTrackRange()`.
* But I guess it makes sense: It is a function that is called every tick for each entity, and it is a bit wasteful "converting" (in quotes, because it is actually accessing `int ordinal`) from enum to ordinal every time.
* Cache coordinate key used for nearby players when ticking chunks * Cache coordinate key used for nearby players when ticking chunks
* The `getChunkKey(...)` call is a bit expensive, using 0.24% of CPU time with 19k chunks loaded. * 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. * So instead of paying the price on each tick, we pay the price when the chunk is loaded.
@@ -64,7 +61,6 @@ SparklyPaper's config file is `sparklypaper.yml`, the file is, by default, place
* 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. * 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. * 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. * 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!).
* Cache last `shouldTickBlocksAt` result when ticking block entities * Cache last `shouldTickBlocksAt` result when ticking block entities
* The `shouldTickBlocksAt` call is expensive, it requires pulling chunk holder info from an map for each block entity (even if the block entities are on the same chunk!) every single time * The `shouldTickBlocksAt` call is expensive, it requires pulling chunk holder info from an map for each block entity (even if the block entities are on the same chunk!) every single time
* So here's a quick and dirty optimization: We cache the last `shouldTickBlocksAt` result and, if the last chunk position is the same as our cached value, we use the last cached `shouldTickBlocksAt` result! * So here's a quick and dirty optimization: We cache the last `shouldTickBlocksAt` result and, if the last chunk position is the same as our cached value, we use the last cached `shouldTickBlocksAt` result!

View File

@@ -1,55 +0,0 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: MrPowerGamerBR <git@mrpowergamerbr.com>
Date: Wed, 22 Nov 2023 01:44:52 -0300
Subject: [PATCH] Cache tracking range type enum ordinal
Yes, I was shocked, flabbergasted even, when I found out that "Enum.ordinal()" was lagging here
But I guess it makes sense: It is a function that is called every tick for each entity, and it is a bit wasteful converting from enum to ordinal every time
diff --git a/src/main/java/net/minecraft/world/entity/Entity.java b/src/main/java/net/minecraft/world/entity/Entity.java
index e544081e8214802facb77defc1e9aa765834be2a..6d9aa481cf0a3c31505977b98ca5f2a6b812a757 100644
--- a/src/main/java/net/minecraft/world/entity/Entity.java
+++ b/src/main/java/net/minecraft/world/entity/Entity.java
@@ -501,6 +501,7 @@ public abstract class Entity implements Nameable, EntityAccess, CommandSource {
// Paper end
// Paper start - optimise entity tracking
final org.spigotmc.TrackingRange.TrackingRangeType trackingRangeType = org.spigotmc.TrackingRange.getTrackingRangeType(this);
+ final int trackingRangeTypeOrdinal = trackingRangeType.ordinal(); // SparklyPaper - cache tracking range type enum ordinal
// Paper start - make end portalling safe
public BlockPos portalBlock;
public ServerLevel portalWorld;
@@ -541,24 +542,26 @@ public abstract class Entity implements Nameable, EntityAccess, CommandSource {
public final com.destroystokyo.paper.util.misc.PooledLinkedHashSets.PooledObjectLinkedOpenHashSet<ServerPlayer> getPlayersInTrackRange() {
// determine highest range of passengers
if (this.passengers.isEmpty()) {
- return ((ServerLevel)this.level).getChunkSource().chunkMap.playerEntityTrackerTrackMaps[this.trackingRangeType.ordinal()]
+ return ((ServerLevel)this.level).getChunkSource().chunkMap.playerEntityTrackerTrackMaps[this.trackingRangeTypeOrdinal] // SparklyPaper - cache tracking range type enum ordinal
.getObjectsInRange(MCUtil.getCoordinateKey(this));
}
Iterable<Entity> passengers = this.getIndirectPassengers();
net.minecraft.server.level.ChunkMap chunkMap = ((ServerLevel)this.level).getChunkSource().chunkMap;
- org.spigotmc.TrackingRange.TrackingRangeType type = this.trackingRangeType;
- int range = chunkMap.getEntityTrackerRange(type.ordinal());
+ int typeOrdinal = this.trackingRangeTypeOrdinal; // SparklyPaper - cache tracking range type enum ordinal
+ int range = chunkMap.getEntityTrackerRange(typeOrdinal); // SparklyPaper - cache tracking range type enum ordinal
for (Entity passenger : passengers) {
- org.spigotmc.TrackingRange.TrackingRangeType passengerType = passenger.trackingRangeType;
- int passengerRange = chunkMap.getEntityTrackerRange(passengerType.ordinal());
+ // SparklyPaper start - cache tracking range type enum ordinal
+ int passengerTypeOrdinal = passenger.trackingRangeTypeOrdinal;
+ int passengerRange = chunkMap.getEntityTrackerRange(passengerTypeOrdinal);
if (passengerRange > range) {
- type = passengerType;
+ typeOrdinal = passengerTypeOrdinal; // SparklyPaper - cache tracking range type enum ordinal
range = passengerRange;
}
+ // SparklyPaper end
}
- return chunkMap.playerEntityTrackerTrackMaps[type.ordinal()].getObjectsInRange(MCUtil.getCoordinateKey(this));
+ return chunkMap.playerEntityTrackerTrackMaps[typeOrdinal].getObjectsInRange(MCUtil.getCoordinateKey(this)); // SparklyPaper - cache tracking range type enum ordinal
}
// Paper end - optimise entity tracking

View File

@@ -14,36 +14,20 @@ This seems stupid, but it does seem that it improves the performance a bit, and
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 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 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 index caa73632aee15583c6b6ed12a668c8f49b794708..fa4c8a52a57775ef8f23e48e57b76ff7abc370c0 100644
--- a/src/main/java/net/minecraft/server/level/ChunkMap.java --- a/src/main/java/net/minecraft/server/level/ChunkMap.java
+++ b/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 @@ -1436,7 +1436,7 @@ public class ChunkMap extends ChunkStorage implements ChunkHolder.PlayerProvider
private final int range;
SectionPos lastSectionPos;
public final Set<ServerPlayerConnection> 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<ServerPlayer> newTrackerCandidates) {
com.destroystokyo.paper.util.misc.PooledLinkedHashSets.PooledObjectLinkedOpenHashSet<ServerPlayer> 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 // Paper end - check Y
// CraftBukkit start - respect vanish API // CraftBukkit start - respect vanish API
- if (flag && !player.getBukkitEntity().canSee(this.entity.getBukkitEntity())) { // Paper - only consider hits - 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 + if (flag && !player.getBukkitEntity().canSeeChunkMapUpdatePlayer(this.entity.getBukkitEntity())) { // Paper - only consider hits // SparklyPaper - optimize canSee checks
flag = false; flag = false;
} }
// CraftBukkit end // CraftBukkit end
diff --git a/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java b/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java 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 index 83aaf3e6e377d731ce02f779f80b7bf5db46f89f..1424783f9ebf44d260721fb36c6ae90adee82e1d 100644
--- a/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java --- a/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java
+++ b/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 { @@ -183,7 +183,7 @@ public class CraftPlayer extends CraftHumanEntity implements Player {
@@ -65,8 +49,8 @@ index 83aaf3e6e377d731ce02f779f80b7bf5db46f89f..3a02f92b57563b42680f4b867d681d22
+ // 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 + // 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) { + public boolean canSeeChunkMapUpdatePlayer(org.bukkit.entity.Entity entity) {
+ return visibleByDefault ^ (!invertedVisibilityEntities.isEmpty() && this.invertedVisibilityEntities.containsKey(entity.getUniqueId())); // SPIGOT-7312: Can always see self // SparklyPaper - optimize canSee checks + return entity.isVisibleByDefault() ^ (!invertedVisibilityEntities.isEmpty() && this.invertedVisibilityEntities.containsKey(entity.getUniqueId())); // SPIGOT-7312: Can always see self // SparklyPaper - optimize canSee checks
+ } + }
+ // SparklyPaper end + // SparklyPaper end
+ +

View File

@@ -1083,10 +1083,10 @@ index 33abcf12b4426572b74ca4c813e4392c823494bc..07198f2f8f7cb082c9e575a5c1e56c14
entityplayer1.connection = entityplayer.connection; entityplayer1.connection = entityplayer.connection;
diff --git a/src/main/java/net/minecraft/world/entity/Entity.java b/src/main/java/net/minecraft/world/entity/Entity.java diff --git a/src/main/java/net/minecraft/world/entity/Entity.java b/src/main/java/net/minecraft/world/entity/Entity.java
index 136ba5d75cadb3ed3e3b70048d3509d57d050651..052c26a0bb9181407c33204d1fd80d80c3df59fe 100644 index e544081e8214802facb77defc1e9aa765834be2a..f979d22f5bf83492133a87119686a4e136923bc0 100644
--- a/src/main/java/net/minecraft/world/entity/Entity.java --- a/src/main/java/net/minecraft/world/entity/Entity.java
+++ b/src/main/java/net/minecraft/world/entity/Entity.java +++ b/src/main/java/net/minecraft/world/entity/Entity.java
@@ -937,11 +937,11 @@ public abstract class Entity implements Nameable, EntityAccess, CommandSource { @@ -934,11 +934,11 @@ public abstract class Entity implements Nameable, EntityAccess, CommandSource {
// This will be called every single tick the entity is in lava, so don't throw an event // This will be called every single tick the entity is in lava, so don't throw an event
this.setSecondsOnFire(15, false); this.setSecondsOnFire(15, false);
} }
@@ -1100,7 +1100,7 @@ index 136ba5d75cadb3ed3e3b70048d3509d57d050651..052c26a0bb9181407c33204d1fd80d80
// CraftBukkit end - we also don't throw an event unless the object in lava is living, to save on some event calls // CraftBukkit end - we also don't throw an event unless the object in lava is living, to save on some event calls
} }
@@ -3393,9 +3393,9 @@ public abstract class Entity implements Nameable, EntityAccess, CommandSource { @@ -3390,9 +3390,9 @@ public abstract class Entity implements Nameable, EntityAccess, CommandSource {
if (this.fireImmune()) { if (this.fireImmune()) {
return; return;
} }
@@ -1112,7 +1112,7 @@ index 136ba5d75cadb3ed3e3b70048d3509d57d050651..052c26a0bb9181407c33204d1fd80d80
return; return;
} }
// CraftBukkit end // CraftBukkit end
@@ -3906,6 +3906,7 @@ public abstract class Entity implements Nameable, EntityAccess, CommandSource { @@ -3903,6 +3903,7 @@ public abstract class Entity implements Nameable, EntityAccess, CommandSource {
this.teleportPassengers(); this.teleportPassengers();
this.setYHeadRot(yaw); this.setYHeadRot(yaw);
} else { } else {
@@ -1322,7 +1322,7 @@ index 45243249a561440512ef2a620c60b02e159c80e2..849b9b4336d2ac99324dacf6ad8a2e34
continue; continue;
} }
diff --git a/src/main/java/net/minecraft/world/level/Level.java b/src/main/java/net/minecraft/world/level/Level.java diff --git a/src/main/java/net/minecraft/world/level/Level.java b/src/main/java/net/minecraft/world/level/Level.java
index 14936213cf2a9f0d7f4d0fbf7387e6d97c6cf717..ef0776b6429f1e82c7bd29b19b0328722b426a18 100644 index b008bbc1bfef5d2ea53ed2fbffa505a398b9f26c..581021896bfc143c5e3d669a3e4ecb9184eb793a 100644
--- a/src/main/java/net/minecraft/world/level/Level.java --- a/src/main/java/net/minecraft/world/level/Level.java
+++ b/src/main/java/net/minecraft/world/level/Level.java +++ b/src/main/java/net/minecraft/world/level/Level.java
@@ -15,6 +15,8 @@ import java.util.function.Consumer; @@ -15,6 +15,8 @@ import java.util.function.Consumer;