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

Split the tickBlockEntities optimization into two patches

This commit is contained in:
MrPowerGamerBR
2023-11-26 14:15:51 -03:00
parent d8ba56e45b
commit b82e1078fe
7 changed files with 330 additions and 267 deletions

View File

@@ -58,10 +58,12 @@ 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.
* 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.
* Optimize `tickBlockEntities`
* Fix `MC-117075`: TE Unload Lag Spike
* We replaced the `blockEntityTickers` list with a custom list based on fastutil's `ObjectArrayList` with a small yet huge change for us: A method that allows us to remove a list of indexes from the list.
* This is WAY FASTER than using `removeAll` with a list of entries to be removed, because we don't need to calculate the identity of each block entity to be removed, and we can jump directly to where the search should begin, giving a performance boost for small removals (because we don't need to loop thru the entire list to find what element should be removed) and a performance boost for big removals (no need to calculate the identity of each block entity).
* We also now cache the last `shouldTickBlocksAt` result, because the `shouldTickBlocksAt` is expensive because 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, if the last chunk position is the same as our cached value, we use the last cached `shouldTickBlocksAt` result!
* This reverts a Paper patch with the same name, since both are editing the same section.
* Optimize `tickBlockEntities`
* We cache the last `shouldTickBlocksAt` result, because the `shouldTickBlocksAt` is expensive because 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, if the last chunk position is the same as our cached value, we use the last cached `shouldTickBlocksAt` result!
* We could use a map for caching, but here's why this is way better than using a map: The block entity ticking list is sorted by chunks! Well, sort of... It is sorted by chunk when the chunk has been loaded, newly placed blocks will be appended to the end of the list until the chunk unloads and loads again. Most block entities are things that players placed to be there for a long time anyway (like hoppers, etc)
* But here's the thing: We don't care if we have a small performance penalty if the players have placed new block entities, the small performance hit of when a player placed new block entities is so small ('tis just a long comparsion after all), that the performance boost from already placed block entities is bigger, this helps a lot if your server has a lot of chunks with multiple block entities, and the block entities will be automatically sorted after the chunk is unloaded and loaded again, so it ain't that bad.
* And finally, we also cache the chunk's coordinate key when creating the block entity, which is actually "free" because we just reuse the already cached chunk coordinate key from the chunk!

View File

@@ -1,257 +0,0 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: MrPowerGamerBR <git@mrpowergamerbr.com>
Date: Thu, 23 Nov 2023 18:05:00 -0300
Subject: [PATCH] Optimize tickBlockEntities
We replaced the "blockEntityTickers" list with a custom list based on fastutil's "ObjectArrayList" with a small yet huge change for us: A method that allows us to remove a list of indexes from the list.
This is WAY FASTER than using "removeAll" with a list of entries to be removed, because we don't need to calculate the identity of each block entity to be removed, and we can jump directly to where the search should begin, giving a performance boost for small removals (because we don't need to loop thru the entire list to find what element should be removed) and a performance boost for big removals (no need to calculate the identity of each block entity).
We also now cache the last "shouldTickBlocksAt" result, because the "shouldTickBlocksAt" is expensive because 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, if the last chunk position is the same as our cached value, we use the last cached "shouldTickBlocksAt" result!
We could use a map for caching, but here's why this is way better than using a map: The block entity ticking list is sorted by chunks! Well, sort of... It is sorted by chunk when the chunk has been loaded, newly placed blocks will be appended to the end of the list until the chunk unloads and loads again. Most block entities are things that players placed to be there for a long time anyway (like hoppers, etc)
But here's the thing: We don't care if we have a small performance penalty if the players have placed new block entities, the small performance hit of when a player placed new block entities is so small ('tis just a long comparsion after all), that the performance boost from already placed block entities is bigger, this helps a lot if your server has a lot of chunks with multiple block entities, and the block entities will be automatically sorted after the chunk is unloaded and loaded again, so it ain't that bad.
And finally, we also cache the chunk's coordinate key when creating the block entity, which is actually "free" because we just reuse the already cached chunk coordinate key from the chunk!
diff --git a/src/main/java/net/minecraft/world/level/Level.java b/src/main/java/net/minecraft/world/level/Level.java
index b9e0822638a3979bd43392efdb595153e6f34675..868e4b69e7ffa502a1ea188053d6f6ae125a554a 100644
--- a/src/main/java/net/minecraft/world/level/Level.java
+++ b/src/main/java/net/minecraft/world/level/Level.java
@@ -117,7 +117,7 @@ public abstract class Level implements LevelAccessor, AutoCloseable {
public static final int TICKS_PER_DAY = 24000;
public static final int MAX_ENTITY_SPAWN_Y = 20000000;
public static final int MIN_ENTITY_SPAWN_Y = -20000000;
- protected final List<TickingBlockEntity> blockEntityTickers = Lists.newArrayList(); public final int getTotalTileEntityTickers() { return this.blockEntityTickers.size(); } // Paper
+ protected final net.sparklypower.sparklypaper.BlockEntityTickersList blockEntityTickers = new net.sparklypower.sparklypaper.BlockEntityTickersList(); /* Lists.newArrayList(); */ public final int getTotalTileEntityTickers() { return this.blockEntityTickers.size(); } // Paper // SparklyPaper - optimize tickBlockEntities
protected final NeighborUpdater neighborUpdater;
private final List<TickingBlockEntity> pendingBlockEntityTickers = Lists.newArrayList();
private boolean tickingBlockEntities;
@@ -1270,8 +1270,14 @@ public abstract class Level implements LevelAccessor, AutoCloseable {
// Spigot start
// Iterator iterator = this.blockEntityTickers.iterator();
int tilesThisCycle = 0;
- var toRemove = new it.unimi.dsi.fastutil.objects.ObjectOpenCustomHashSet<TickingBlockEntity>(net.minecraft.Util.identityStrategy()); // Paper - use removeAll
- toRemove.add(null);
+ // SparklyPaper start - optimize tickBlockEntities
+ // var toRemove = new it.unimi.dsi.fastutil.objects.ObjectOpenCustomHashSet<TickingBlockEntity>(net.minecraft.Util.identityStrategy()); // Paper - use removeAll
+ // toRemove.add(null);
+ var toRemove = new java.util.HashSet<Integer>(); // For some reason, Java's HashSet seems to be faster than fastutil's only if we are removing HUGE amounts of tile entities, idk why
+ var startSearchFromIndex = -1;
+ var shouldTickBlocksAtLastResult = -1; // -1 = undefined
+ var shouldTickBlocksAtChunkPos = 0L;
+ // SparklyPaper end
for (tileTickPosition = 0; tileTickPosition < this.blockEntityTickers.size(); tileTickPosition++) { // Paper - Disable tick limiters
this.tileTickPosition = (this.tileTickPosition < this.blockEntityTickers.size()) ? this.tileTickPosition : 0;
TickingBlockEntity tickingblockentity = (TickingBlockEntity) this.blockEntityTickers.get(this.tileTickPosition);
@@ -1279,6 +1285,11 @@ public abstract class Level implements LevelAccessor, AutoCloseable {
if (tickingblockentity == null) {
this.getCraftServer().getLogger().severe("Spigot has detected a null entity and has removed it, preventing a crash");
tilesThisCycle--;
+ // SparklyPaper start - optimize tickBlockEntities
+ toRemove.add(tileTickPosition);
+ if (startSearchFromIndex == -1)
+ startSearchFromIndex = tileTickPosition;
+ // SparklyPaper end
continue;
}
// Spigot end
@@ -1286,19 +1297,38 @@ public abstract class Level implements LevelAccessor, AutoCloseable {
if (tickingblockentity.isRemoved()) {
// Spigot start
tilesThisCycle--;
- toRemove.add(tickingblockentity); // Paper - use removeAll
+ // SparklyPaper start - optimize tickBlockEntities
+ // toRemove.add(tickingblockentity); // Paper - use removeAll
+ toRemove.add(tileTickPosition);
+ if (startSearchFromIndex == -1)
+ startSearchFromIndex = tileTickPosition;
+ // SparklyPaper end
// Spigot end
- } else if (this.shouldTickBlocksAt(tickingblockentity.getPos())) {
+ // } else if (this.shouldTickBlocksAt(tickingblockentity.getPos())) { // SparklyPaper start - optimize tickBlockEntities
+ } else {
+ long chunkPos = tickingblockentity.getChunkCoordinateKey();
+ boolean shouldTick;
+ if (shouldTickBlocksAtChunkPos == chunkPos && shouldTickBlocksAtLastResult != -1) {
+ shouldTick = shouldTickBlocksAtLastResult == 1;
+ } else {
+ shouldTick = this.shouldTickBlocksAt(chunkPos);
+ shouldTickBlocksAtLastResult = shouldTick ? 1 : 0;
+ shouldTickBlocksAtChunkPos = chunkPos;
+ }
+ if (shouldTick) {
tickingblockentity.tick();
// Paper start - execute chunk tasks during tick
if ((this.tileTickPosition & 7) == 0) {
MinecraftServer.getServer().executeMidTickTasks();
}
// Paper end - execute chunk tasks during tick
+ } // SparklyPaper end
}
}
- this.blockEntityTickers.removeAll(toRemove);
-
+ // SparklyPaper start - optimize tickBlockEntities
+ // this.blockEntityTickers.removeAll(toRemove);
+ this.blockEntityTickers.removeAllByIndex(startSearchFromIndex, toRemove); // We don't need to care about if the startSearchFromIndex can be -1 here, since if it is -1, then the toRemove list is empty and the call will fast fail
+ // SparklyPaper end
this.timings.tileEntityTick.stopTiming(); // Spigot
this.tickingBlockEntities = false;
co.aikar.timings.TimingHistory.tileEntityTicks += this.blockEntityTickers.size(); // Paper
diff --git a/src/main/java/net/minecraft/world/level/block/entity/TickingBlockEntity.java b/src/main/java/net/minecraft/world/level/block/entity/TickingBlockEntity.java
index 28e3b73507b988f7234cbf29c4024c88180d0aef..68d96dd3e288346d8df49b52fa035e8154057065 100644
--- a/src/main/java/net/minecraft/world/level/block/entity/TickingBlockEntity.java
+++ b/src/main/java/net/minecraft/world/level/block/entity/TickingBlockEntity.java
@@ -10,4 +10,6 @@ public interface TickingBlockEntity {
BlockPos getPos();
String getType();
+
+ long getChunkCoordinateKey(); // SparklyPaper - cache last shouldTickBlocksAt result when ticking block entities
}
diff --git a/src/main/java/net/minecraft/world/level/chunk/LevelChunk.java b/src/main/java/net/minecraft/world/level/chunk/LevelChunk.java
index fa170cc1ce7011d201295b89718292d696c7fc24..c6f62c56da6e74fbaa57300f8cc2718675d1681e 100644
--- a/src/main/java/net/minecraft/world/level/chunk/LevelChunk.java
+++ b/src/main/java/net/minecraft/world/level/chunk/LevelChunk.java
@@ -73,6 +73,13 @@ public class LevelChunk extends ChunkAccess {
public String getType() {
return "<null>";
}
+
+ // SparklyPaper start - cache last shouldTickBlocksAt result when ticking block entities
+ @Override
+ public long getChunkCoordinateKey() {
+ return 0;
+ }
+ // SparklyPaper end
};
private final Map<BlockPos, LevelChunk.RebindableTickingBlockEntityWrapper> tickersInLevel;
public boolean loaded;
@@ -1090,7 +1097,7 @@ public class LevelChunk extends ChunkAccess {
}
private <T extends BlockEntity> TickingBlockEntity createTicker(T blockEntity, BlockEntityTicker<T> blockEntityTicker) {
- return new LevelChunk.BoundTickingBlockEntity<>(blockEntity, blockEntityTicker);
+ return new LevelChunk.BoundTickingBlockEntity<>(blockEntity, blockEntityTicker, coordinateKey); // SparklyPaper - cache last shouldTickBlocksAt result when ticking block entities
}
@FunctionalInterface
@@ -1141,6 +1148,13 @@ public class LevelChunk extends ChunkAccess {
public String toString() {
return this.ticker + " <wrapped>";
}
+
+ // SparklyPaper start - cache last shouldTickBlocksAt result when ticking block entities
+ @Override
+ public long getChunkCoordinateKey() {
+ return this.ticker.getChunkCoordinateKey();
+ }
+ // SparklyPaper end
}
private class BoundTickingBlockEntity<T extends BlockEntity> implements TickingBlockEntity {
@@ -1148,10 +1162,12 @@ public class LevelChunk extends ChunkAccess {
private final T blockEntity;
private final BlockEntityTicker<T> ticker;
private boolean loggedInvalidBlockState;
+ private final long chunkCoordinateKey; // SparklyPaper - cache last shouldTickBlocksAt result when ticking block entities
- BoundTickingBlockEntity(BlockEntity tileentity, BlockEntityTicker blockentityticker) {
+ BoundTickingBlockEntity(BlockEntity tileentity, BlockEntityTicker blockentityticker, long chunkCoordinateKey) { // SparklyPaper - cache last shouldTickBlocksAt result when ticking block entities
this.blockEntity = (T) tileentity; // CraftBukkit - decompile error
this.ticker = blockentityticker;
+ this.chunkCoordinateKey = chunkCoordinateKey; // SparklyPaper - cache last shouldTickBlocksAt result when ticking block entities
}
@Override
@@ -1214,5 +1230,12 @@ public class LevelChunk extends ChunkAccess {
return "Level ticker for " + s + "@" + this.getPos();
}
+
+ // SparklyPaper start - cache last shouldTickBlocksAt result when ticking block entities
+ @Override
+ public long getChunkCoordinateKey() {
+ return this.chunkCoordinateKey;
+ }
+ // SparklyPaper end
}
}
diff --git a/src/main/java/net/sparklypower/sparklypaper/BlockEntityTickersList.java b/src/main/java/net/sparklypower/sparklypaper/BlockEntityTickersList.java
new file mode 100644
index 0000000000000000000000000000000000000000..f2c00eb6092be5f2f9b7ea23cc45061135f0d1fa
--- /dev/null
+++ b/src/main/java/net/sparklypower/sparklypaper/BlockEntityTickersList.java
@@ -0,0 +1,70 @@
+package net.sparklypower.sparklypaper;
+
+import it.unimi.dsi.fastutil.objects.ObjectArrayList;
+import net.minecraft.world.level.block.entity.TickingBlockEntity;
+
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Set;
+
+/**
+ * A list for ServerLevel's blockEntityTickers
+ *
+ * This list is behaves identically to ObjectArrayList, but it has an additional method, `removeAllByIndex`, that allows a list of integers to be passed indicating what
+ * indexes should be deleted from the list
+ *
+ * This is faster than using removeAll, since we don't need to compare the identity of each block entity, and faster than looping thru each index manually and deleting with remove,
+ * since we don't need to resize the array every single remove.
+ */
+public class BlockEntityTickersList extends ObjectArrayList<TickingBlockEntity> {
+ /** Creates a new array list with {@link #DEFAULT_INITIAL_CAPACITY} capacity. */
+ public BlockEntityTickersList() {
+ super();
+ }
+
+ /**
+ * Creates a new array list and fills it with a given collection.
+ *
+ * @param c a collection that will be used to fill the array list.
+ */
+ public BlockEntityTickersList(final Collection<? extends TickingBlockEntity> c) {
+ super(c);
+ }
+
+ /**
+ * Removes elements by their index.
+ */
+ public boolean removeAllByIndex(final int startSearchFromIndex, final Set<Integer> c) {
+ final int requiredMatches = c.size();
+ if (requiredMatches == 0)
+ return false; // exit early, we don't need to do anything
+
+ final Object[] a = this.a;
+ int j = startSearchFromIndex;
+ int matches = 0;
+ for (int i = startSearchFromIndex; i < size; i++) { // If the user knows the first index to be removed, we can skip a lot of unnecessary comparsions
+ if (!c.contains(i)) {
+ a[j++] = a[i];
+ } else {
+ matches++;
+ }
+
+ if (matches == requiredMatches) { // Exit the loop if we already removed everything, we don't need to check anything else
+ // We need to update the final size here, because we know that we already found everything!
+ // Because we know that the size must be currentSize - requiredMatches (because we have matched everything), let's update the value
+ // However, we need to copy the rest of the stuff over
+ if (i != (size - 1)) { // If it isn't the last index...
+ // i + 1 because we want to copy the *next* element over
+ // and the size - i - 1 is because we want to get the current size, minus the current index (which is i), and then - 1 because we want to copy -1 ahead (remember, we are adding +1 to copy the *next* element)
+ System.arraycopy(a, i + 1, a, j, size - i - 1);
+ }
+ j = size - requiredMatches;
+ break;
+ }
+ }
+ Arrays.fill(a, j, size, null);
+ final boolean modified = size != j;
+ size = j;
+ return modified;
+ }
+}

View File

@@ -0,0 +1,45 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: MrPowerGamerBR <git@mrpowergamerbr.com>
Date: Sun, 26 Nov 2023 12:52:34 -0300
Subject: [PATCH] Revert "Fix MC-117075: TE Unload Lag Spike"
This reverts commit 188c1c5b77133f7c3da9c67a97432d79d50d2b34.
diff --git a/src/main/java/net/minecraft/world/level/Level.java b/src/main/java/net/minecraft/world/level/Level.java
index b9e0822638a3979bd43392efdb595153e6f34675..ddb618fce875b1a337b139c9c47433453654017b 100644
--- a/src/main/java/net/minecraft/world/level/Level.java
+++ b/src/main/java/net/minecraft/world/level/Level.java
@@ -1270,8 +1270,6 @@ public abstract class Level implements LevelAccessor, AutoCloseable {
// Spigot start
// Iterator iterator = this.blockEntityTickers.iterator();
int tilesThisCycle = 0;
- var toRemove = new it.unimi.dsi.fastutil.objects.ObjectOpenCustomHashSet<TickingBlockEntity>(net.minecraft.Util.identityStrategy()); // Paper - use removeAll
- toRemove.add(null);
for (tileTickPosition = 0; tileTickPosition < this.blockEntityTickers.size(); tileTickPosition++) { // Paper - Disable tick limiters
this.tileTickPosition = (this.tileTickPosition < this.blockEntityTickers.size()) ? this.tileTickPosition : 0;
TickingBlockEntity tickingblockentity = (TickingBlockEntity) this.blockEntityTickers.get(this.tileTickPosition);
@@ -1279,6 +1277,7 @@ public abstract class Level implements LevelAccessor, AutoCloseable {
if (tickingblockentity == null) {
this.getCraftServer().getLogger().severe("Spigot has detected a null entity and has removed it, preventing a crash");
tilesThisCycle--;
+ this.blockEntityTickers.remove(this.tileTickPosition--);
continue;
}
// Spigot end
@@ -1286,7 +1285,7 @@ public abstract class Level implements LevelAccessor, AutoCloseable {
if (tickingblockentity.isRemoved()) {
// Spigot start
tilesThisCycle--;
- toRemove.add(tickingblockentity); // Paper - use removeAll
+ this.blockEntityTickers.remove(this.tileTickPosition--);
// Spigot end
} else if (this.shouldTickBlocksAt(tickingblockentity.getPos())) {
tickingblockentity.tick();
@@ -1297,7 +1296,6 @@ public abstract class Level implements LevelAccessor, AutoCloseable {
// Paper end - execute chunk tasks during tick
}
}
- this.blockEntityTickers.removeAll(toRemove);
this.timings.tileEntityTick.stopTiming(); // Spigot
this.tickingBlockEntities = false;

View File

@@ -0,0 +1,150 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: MrPowerGamerBR <git@mrpowergamerbr.com>
Date: Sun, 26 Nov 2023 13:02:16 -0300
Subject: [PATCH] Fix MC-117075: TE Unload Lag Spike
We replaced the `blockEntityTickers` list with a custom list based on fastutil's `ObjectArrayList` with a small yet huge change for us: A method that allows us to remove a list of indexes from the list.
This is WAY FASTER than using `removeAll` with a list of entries to be removed, because we don't need to calculate the identity of each block entity to be removed, and we can jump directly to where the search should begin, giving a performance boost for small removals (because we don't need to loop thru the entire list to find what element should be removed) and a performance boost for big removals (no need to calculate the identity of each block entity).
diff --git a/src/main/java/net/minecraft/world/level/Level.java b/src/main/java/net/minecraft/world/level/Level.java
index ddb618fce875b1a337b139c9c47433453654017b..36c1cc48e6c2541d1218db0560ed7b315ce8067d 100644
--- a/src/main/java/net/minecraft/world/level/Level.java
+++ b/src/main/java/net/minecraft/world/level/Level.java
@@ -117,7 +117,7 @@ public abstract class Level implements LevelAccessor, AutoCloseable {
public static final int TICKS_PER_DAY = 24000;
public static final int MAX_ENTITY_SPAWN_Y = 20000000;
public static final int MIN_ENTITY_SPAWN_Y = -20000000;
- protected final List<TickingBlockEntity> blockEntityTickers = Lists.newArrayList(); public final int getTotalTileEntityTickers() { return this.blockEntityTickers.size(); } // Paper
+ protected final net.sparklypower.sparklypaper.BlockEntityTickersList blockEntityTickers = new net.sparklypower.sparklypaper.BlockEntityTickersList(); public final int getTotalTileEntityTickers() { return this.blockEntityTickers.size(); } // Paper // SparklyPaper - optimize block entity removals
protected final NeighborUpdater neighborUpdater;
private final List<TickingBlockEntity> pendingBlockEntityTickers = Lists.newArrayList();
private boolean tickingBlockEntities;
@@ -1277,7 +1277,7 @@ public abstract class Level implements LevelAccessor, AutoCloseable {
if (tickingblockentity == null) {
this.getCraftServer().getLogger().severe("Spigot has detected a null entity and has removed it, preventing a crash");
tilesThisCycle--;
- this.blockEntityTickers.remove(this.tileTickPosition--);
+ blockEntityTickers.markAsRemoved(this.tileTickPosition); // this.blockEntityTickers.remove(this.tileTickPosition--); // SparklyPaper - optimize block entity removals
continue;
}
// Spigot end
@@ -1285,7 +1285,7 @@ public abstract class Level implements LevelAccessor, AutoCloseable {
if (tickingblockentity.isRemoved()) {
// Spigot start
tilesThisCycle--;
- this.blockEntityTickers.remove(this.tileTickPosition--);
+ blockEntityTickers.markAsRemoved(this.tileTickPosition); // this.blockEntityTickers.remove(this.tileTickPosition--); // SparklyPaper - optimize block entity removals
// Spigot end
} else if (this.shouldTickBlocksAt(tickingblockentity.getPos())) {
tickingblockentity.tick();
@@ -1296,7 +1296,7 @@ public abstract class Level implements LevelAccessor, AutoCloseable {
// Paper end - execute chunk tasks during tick
}
}
-
+ blockEntityTickers.removeMarkedEntries(); // SparklyPaper - optimize block entity removals
this.timings.tileEntityTick.stopTiming(); // Spigot
this.tickingBlockEntities = false;
co.aikar.timings.TimingHistory.tileEntityTicks += this.blockEntityTickers.size(); // Paper
diff --git a/src/main/java/net/sparklypower/sparklypaper/BlockEntityTickersList.java b/src/main/java/net/sparklypower/sparklypaper/BlockEntityTickersList.java
new file mode 100644
index 0000000000000000000000000000000000000000..e00d27afc53f4d0f5bacdd81e06b81d231e512ff
--- /dev/null
+++ b/src/main/java/net/sparklypower/sparklypaper/BlockEntityTickersList.java
@@ -0,0 +1,95 @@
+package net.sparklypower.sparklypaper;
+
+import it.unimi.dsi.fastutil.objects.ObjectArrayList;
+import net.minecraft.world.level.block.entity.TickingBlockEntity;
+
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.Set;
+
+/**
+ + * A list for ServerLevel's blockEntityTickers
+ + *
+ + * This list is behaves identically to ObjectArrayList, but it has an additional method, `removeAllByIndex`, that allows a list of integers to be passed indicating what
+ + * indexes should be deleted from the list
+ + *
+ + * This is faster than using removeAll, since we don't need to compare the identity of each block entity, and faster than looping thru each index manually and deleting with remove,
+ + * since we don't need to resize the array every single remove.
+ + */
+public class BlockEntityTickersList extends ObjectArrayList<TickingBlockEntity> {
+ private final HashSet<Integer> toRemove = new HashSet<>(); // For some reason, Java's HashSet seems to be faster than fastutil's only if we are removing HUGE amounts of tile entities, idk why
+ private int startSearchFromIndex = Integer.MAX_VALUE;
+
+ /** Creates a new array list with {@link #DEFAULT_INITIAL_CAPACITY} capacity. */
+ public BlockEntityTickersList() {
+ super();
+ }
+
+ /**
+ * Creates a new array list and fills it with a given collection.
+ *
+ * @param c a collection that will be used to fill the array list.
+ */
+ public BlockEntityTickersList(final Collection<? extends TickingBlockEntity> c) {
+ super(c);
+ }
+
+ /**
+ * Marks an entry as removed
+ *
+ * @param index
+ */
+ public void markAsRemoved(final int index) {
+ if (this.startSearchFromIndex > index) {
+ this.startSearchFromIndex = index;
+ }
+ this.toRemove.add(index);
+ }
+
+ /**
+ * Removes elements that have been marked as removed.
+ */
+ public void removeMarkedEntries() {
+ if (this.startSearchFromIndex == Integer.MAX_VALUE) // No entries in the list, skip
+ return;
+
+ removeAllByIndex(startSearchFromIndex, toRemove);
+ toRemove.clear();
+ this.startSearchFromIndex = Integer.MAX_VALUE; // Reset the start search index
+ }
+
+ public boolean removeAllByIndex(final int startSearchFromIndex, final Set<Integer> c) {
+ final int requiredMatches = c.size();
+ if (requiredMatches == 0)
+ return false; // exit early, we don't need to do anything
+
+ final Object[] a = this.a;
+ int j = startSearchFromIndex;
+ int matches = 0;
+ for (int i = startSearchFromIndex; i < size; i++) { // If the user knows the first index to be removed, we can skip a lot of unnecessary comparsions
+ if (!c.contains(i)) {
+ a[j++] = a[i];
+ } else {
+ matches++;
+ }
+
+ if (matches == requiredMatches) { // Exit the loop if we already removed everything, we don't need to check anything else
+ // We need to update the final size here, because we know that we already found everything!
+ // Because we know that the size must be currentSize - requiredMatches (because we have matched everything), let's update the value
+ // However, we need to copy the rest of the stuff over
+ if (i != (size - 1)) { // If it isn't the last index...
+ // i + 1 because we want to copy the *next* element over
+ // and the size - i - 1 is because we want to get the current size, minus the current index (which is i), and then - 1 because we want to copy -1 ahead (remember, we are adding +1 to copy the *next* element)
+ System.arraycopy(a, i + 1, a, j, size - i - 1);
+ }
+ j = size - requiredMatches;
+ break;
+ }
+ }
+ Arrays.fill(a, j, size, null);
+ final boolean modified = size != j;
+ size = j;
+ return modified;
+ }
+}

View File

@@ -0,0 +1,123 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: MrPowerGamerBR <git@mrpowergamerbr.com>
Date: Sun, 26 Nov 2023 13:11:10 -0300
Subject: [PATCH] Optimize tickBlockEntities
We cache the last `shouldTickBlocksAt` result, because the `shouldTickBlocksAt` is expensive because 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, if the last chunk position is the same as our cached value, we use the last cached `shouldTickBlocksAt` result!
We could use a map for caching, but here's why this is way better than using a map: The block entity ticking list is sorted by chunks! Well, sort of... It is sorted by chunk when the chunk has been loaded, newly placed blocks will be appended to the end of the list until the chunk unloads and loads again. Most block entities are things that players placed to be there for a long time anyway (like hoppers, etc)
But here's the thing: We don't care if we have a small performance penalty if the players have placed new block entities, the small performance hit of when a player placed new block entities is so small ('tis just a long comparsion after all), that the performance boost from already placed block entities is bigger, this helps a lot if your server has a lot of chunks with multiple block entities, and the block entities will be automatically sorted after the chunk is unloaded and loaded again, so it ain't that bad.
And finally, we also cache the chunk's coordinate key when creating the block entity, which is actually "free" because we just reuse the already cached chunk coordinate key from the chunk!
diff --git a/src/main/java/net/minecraft/world/level/Level.java b/src/main/java/net/minecraft/world/level/Level.java
index 36c1cc48e6c2541d1218db0560ed7b315ce8067d..a6b47d5d0b9b844e76eae9f083569288e6716563 100644
--- a/src/main/java/net/minecraft/world/level/Level.java
+++ b/src/main/java/net/minecraft/world/level/Level.java
@@ -1270,6 +1270,10 @@ public abstract class Level implements LevelAccessor, AutoCloseable {
// Spigot start
// Iterator iterator = this.blockEntityTickers.iterator();
int tilesThisCycle = 0;
+ // SparklyPaper start - optimize tickBlockEntities
+ int shouldTickBlocksAtLastResult = -1; // -1 = undefined
+ long shouldTickBlocksAtChunkPos = 0L;
+ // SparklyPaper end
for (tileTickPosition = 0; tileTickPosition < this.blockEntityTickers.size(); tileTickPosition++) { // Paper - Disable tick limiters
this.tileTickPosition = (this.tileTickPosition < this.blockEntityTickers.size()) ? this.tileTickPosition : 0;
TickingBlockEntity tickingblockentity = (TickingBlockEntity) this.blockEntityTickers.get(this.tileTickPosition);
@@ -1287,13 +1291,25 @@ public abstract class Level implements LevelAccessor, AutoCloseable {
tilesThisCycle--;
blockEntityTickers.markAsRemoved(this.tileTickPosition); // this.blockEntityTickers.remove(this.tileTickPosition--); // SparklyPaper - optimize block entity removals
// Spigot end
- } else if (this.shouldTickBlocksAt(tickingblockentity.getPos())) {
+ // } else if (this.shouldTickBlocksAt(tickingblockentity.getPos())) { // SparklyPaper start - optimize tickBlockEntities
+ } else {
+ long chunkPos = tickingblockentity.getChunkCoordinateKey();
+ boolean shouldTick;
+ if (shouldTickBlocksAtChunkPos == chunkPos && shouldTickBlocksAtLastResult != -1) {
+ shouldTick = shouldTickBlocksAtLastResult == 1;
+ } else {
+ shouldTick = this.shouldTickBlocksAt(chunkPos);
+ shouldTickBlocksAtLastResult = shouldTick ? 1 : 0;
+ shouldTickBlocksAtChunkPos = chunkPos;
+ }
+ if (shouldTick) {
tickingblockentity.tick();
// Paper start - execute chunk tasks during tick
if ((this.tileTickPosition & 7) == 0) {
MinecraftServer.getServer().executeMidTickTasks();
}
// Paper end - execute chunk tasks during tick
+ } // SparklyPaper end
}
}
blockEntityTickers.removeMarkedEntries(); // SparklyPaper - optimize block entity removals
diff --git a/src/main/java/net/minecraft/world/level/chunk/LevelChunk.java b/src/main/java/net/minecraft/world/level/chunk/LevelChunk.java
index fa170cc1ce7011d201295b89718292d696c7fc24..1338f91a85f5db6ce78705a0c48bec8a449a6220 100644
--- a/src/main/java/net/minecraft/world/level/chunk/LevelChunk.java
+++ b/src/main/java/net/minecraft/world/level/chunk/LevelChunk.java
@@ -73,6 +73,13 @@ public class LevelChunk extends ChunkAccess {
public String getType() {
return "<null>";
}
+
+ // SparklyPaper start - optimize tickBlockEntities
+ @Override
+ public long getChunkCoordinateKey() {
+ return 0;
+ }
+ // SparklyPaper end
};
private final Map<BlockPos, LevelChunk.RebindableTickingBlockEntityWrapper> tickersInLevel;
public boolean loaded;
@@ -1090,7 +1097,7 @@ public class LevelChunk extends ChunkAccess {
}
private <T extends BlockEntity> TickingBlockEntity createTicker(T blockEntity, BlockEntityTicker<T> blockEntityTicker) {
- return new LevelChunk.BoundTickingBlockEntity<>(blockEntity, blockEntityTicker);
+ return new LevelChunk.BoundTickingBlockEntity<>(blockEntity, blockEntityTicker, this.coordinateKey); // SparklyPaper - optimize tickBlockEntities
}
@FunctionalInterface
@@ -1141,6 +1148,13 @@ public class LevelChunk extends ChunkAccess {
public String toString() {
return this.ticker + " <wrapped>";
}
+
+ // SparklyPaper start - optimize tickBlockEntities
+ @Override
+ public long getChunkCoordinateKey() {
+ return this.ticker.getChunkCoordinateKey();
+ }
+ // SparklyPaper end
}
private class BoundTickingBlockEntity<T extends BlockEntity> implements TickingBlockEntity {
@@ -1148,10 +1162,12 @@ public class LevelChunk extends ChunkAccess {
private final T blockEntity;
private final BlockEntityTicker<T> ticker;
private boolean loggedInvalidBlockState;
+ private final long chunkCoordinateKey; // SparklyPaper - optimize tickBlockEntities
- BoundTickingBlockEntity(BlockEntity tileentity, BlockEntityTicker blockentityticker) {
+ BoundTickingBlockEntity(BlockEntity tileentity, BlockEntityTicker blockentityticker, long chunkCoordinateKey) { // SparklyPaper - optimize tickBlockEntities
this.blockEntity = (T) tileentity; // CraftBukkit - decompile error
this.ticker = blockentityticker;
+ this.chunkCoordinateKey = chunkCoordinateKey; // SparklyPaper - optimize tickBlockEntities
}
@Override
@@ -1214,5 +1230,12 @@ public class LevelChunk extends ChunkAccess {
return "Level ticker for " + s + "@" + this.getPos();
}
+
+ // SparklyPaper start - optimize tickBlockEntities
+ @Override
+ public long getChunkCoordinateKey() {
+ return this.chunkCoordinateKey;
+ }
+ // SparklyPaper end
}
}

View File

@@ -1322,7 +1322,7 @@ index 45243249a561440512ef2a620c60b02e159c80e2..849b9b4336d2ac99324dacf6ad8a2e34
continue;
}
diff --git a/src/main/java/net/minecraft/world/level/Level.java b/src/main/java/net/minecraft/world/level/Level.java
index 868e4b69e7ffa502a1ea188053d6f6ae125a554a..94eac6837c06e6fd192c108632f1e365a008d6ad 100644
index a6b47d5d0b9b844e76eae9f083569288e6716563..d82fb6b2f0bab36a913ea5e224fb39e6d8eccda2 100644
--- a/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;
@@ -1351,7 +1351,7 @@ index 868e4b69e7ffa502a1ea188053d6f6ae125a554a..94eac6837c06e6fd192c108632f1e365
// CraftBukkit start - tree generation
if (this.captureTreeGeneration) {
// Paper start
@@ -1319,7 +1322,7 @@ public abstract class Level implements LevelAccessor, AutoCloseable {
@@ -1306,7 +1309,7 @@ public abstract class Level implements LevelAccessor, AutoCloseable {
tickingblockentity.tick();
// Paper start - execute chunk tasks during tick
if ((this.tileTickPosition & 7) == 0) {
@@ -1360,7 +1360,7 @@ index 868e4b69e7ffa502a1ea188053d6f6ae125a554a..94eac6837c06e6fd192c108632f1e365
}
// Paper end - execute chunk tasks during tick
} // SparklyPaper end
@@ -1339,7 +1342,7 @@ public abstract class Level implements LevelAccessor, AutoCloseable {
@@ -1323,7 +1326,7 @@ public abstract class Level implements LevelAccessor, AutoCloseable {
public <T extends Entity> void guardEntityTick(Consumer<T> tickConsumer, T entity) {
try {
tickConsumer.accept(entity);
@@ -1369,7 +1369,7 @@ index 868e4b69e7ffa502a1ea188053d6f6ae125a554a..94eac6837c06e6fd192c108632f1e365
} catch (Throwable throwable) {
if (throwable instanceof ThreadDeath) throw throwable; // Paper
// Paper start - Prevent tile entity and entity crashes
@@ -1439,6 +1442,7 @@ public abstract class Level implements LevelAccessor, AutoCloseable {
@@ -1423,6 +1426,7 @@ public abstract class Level implements LevelAccessor, AutoCloseable {
@Nullable
public BlockEntity getBlockEntity(BlockPos blockposition, boolean validate) {
@@ -1377,7 +1377,7 @@ index 868e4b69e7ffa502a1ea188053d6f6ae125a554a..94eac6837c06e6fd192c108632f1e365
// Paper start - Optimize capturedTileEntities lookup
net.minecraft.world.level.block.entity.BlockEntity blockEntity;
if (!this.capturedTileEntities.isEmpty() && (blockEntity = this.capturedTileEntities.get(blockposition)) != null) {
@@ -1446,10 +1450,11 @@ public abstract class Level implements LevelAccessor, AutoCloseable {
@@ -1430,10 +1434,11 @@ public abstract class Level implements LevelAccessor, AutoCloseable {
}
// Paper end
// CraftBukkit end
@@ -1390,7 +1390,7 @@ index 868e4b69e7ffa502a1ea188053d6f6ae125a554a..94eac6837c06e6fd192c108632f1e365
BlockPos blockposition = blockEntity.getBlockPos();
if (!this.isOutsideBuildHeight(blockposition)) {
@@ -1535,6 +1540,7 @@ public abstract class Level implements LevelAccessor, AutoCloseable {
@@ -1519,6 +1524,7 @@ public abstract class Level implements LevelAccessor, AutoCloseable {
@Override
public List<Entity> getEntities(@Nullable Entity except, AABB box, Predicate<? super Entity> predicate) {
@@ -1398,7 +1398,7 @@ index 868e4b69e7ffa502a1ea188053d6f6ae125a554a..94eac6837c06e6fd192c108632f1e365
this.getProfiler().incrementCounter("getEntities");
List<Entity> list = Lists.newArrayList();
((ServerLevel)this).getEntityLookup().getEntities(except, box, list, predicate); // Paper - optimise this call
@@ -1799,8 +1805,7 @@ public abstract class Level implements LevelAccessor, AutoCloseable {
@@ -1783,8 +1789,7 @@ public abstract class Level implements LevelAccessor, AutoCloseable {
}
public final BlockPos.MutableBlockPos getRandomBlockPosition(int x, int y, int z, int l, BlockPos.MutableBlockPos out) {
// Paper end
@@ -1695,7 +1695,7 @@ index a743f36f2682a6b72ffa6644782fc081d1479eb7..85e7da9884f48989a62a789306b701a3
// CraftBukkit end
}
diff --git a/src/main/java/net/minecraft/world/level/chunk/LevelChunk.java b/src/main/java/net/minecraft/world/level/chunk/LevelChunk.java
index c6f62c56da6e74fbaa57300f8cc2718675d1681e..0af3100fdb59028dd43bcfba50c36fca45778fd5 100644
index 1338f91a85f5db6ce78705a0c48bec8a449a6220..00af4b8db53a55d252a43a29c4939fea111d3f13 100644
--- a/src/main/java/net/minecraft/world/level/chunk/LevelChunk.java
+++ b/src/main/java/net/minecraft/world/level/chunk/LevelChunk.java
@@ -419,6 +419,7 @@ public class LevelChunk extends ChunkAccess {