Skip to content

Commit

Permalink
Just use get directly instead of using our hacky hack
Browse files Browse the repository at this point in the history
The real reason I reverted this was because this was throwing an exception, idk why tho because tbh it doesn't make any sense
  • Loading branch information
MrPowerGamerBR committed Nov 25, 2023
1 parent 3b27299 commit f1f4301
Show file tree
Hide file tree
Showing 2 changed files with 99 additions and 14 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: MrPowerGamerBR <[email protected]>
Date: Fri, 24 Nov 2023 23:37:24 -0300
Subject: [PATCH] BlockEntityTickersList optimization tests


diff --git a/src/main/java/net/minecraft/world/level/Level.java b/src/main/java/net/minecraft/world/level/Level.java
index 94eac6837c06e6fd192c108632f1e365a008d6ad..3588657da9969b4207bbeb8109bc101384c03398 100644
--- a/src/main/java/net/minecraft/world/level/Level.java
+++ b/src/main/java/net/minecraft/world/level/Level.java
@@ -1274,8 +1274,8 @@ public abstract class Level implements LevelAccessor, AutoCloseable {
// Iterator iterator = this.blockEntityTickers.iterator();
int tilesThisCycle = 0;
// 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 toRemoveOld = new it.unimi.dsi.fastutil.objects.ObjectOpenCustomHashSet<TickingBlockEntity>(net.minecraft.Util.identityStrategy()); // Paper - use removeAll
+ toRemoveOld.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
@@ -1301,13 +1301,13 @@ public abstract class Level implements LevelAccessor, AutoCloseable {
// Spigot start
tilesThisCycle--;
// SparklyPaper start - optimize tickBlockEntities
- // toRemove.add(tickingblockentity); // Paper - use removeAll
+ toRemoveOld.add(tickingblockentity); // Paper - use removeAll
toRemove.add(tileTickPosition);
if (startSearchFromIndex == -1)
startSearchFromIndex = tileTickPosition;
// SparklyPaper end
// Spigot end
- // } else if (this.shouldTickBlocksAt(tickingblockentity.getPos())) { // SparklyPaper start - optimize tickBlockEntities
+ // } else if (this.shouldTickBlocksAt(tickingblockentity.getPos())) { // SparklyPaper start - optimize tickBlockEntities
} else {
long chunkPos = tickingblockentity.getChunkCoordinateKey();
boolean shouldTick;
@@ -1319,18 +1319,47 @@ public abstract class Level implements LevelAccessor, AutoCloseable {
shouldTickBlocksAtChunkPos = chunkPos;
}
if (shouldTick) {
- tickingblockentity.tick();
- // Paper start - execute chunk tasks during tick
- if ((this.tileTickPosition & 7) == 0) {
- // MinecraftServer.getServer().executeMidTickTasks(); // SparklyPaper - parallel world ticking (only run mid-tick at the end of each tick / fixes concurrency bugs related to executeMidTickTasks)
- }
- // Paper end - execute chunk tasks during tick
+ tickingblockentity.tick();
+ // Paper start - execute chunk tasks during tick
+ if ((this.tileTickPosition & 7) == 0) {
+ // MinecraftServer.getServer().executeMidTickTasks(); // SparklyPaper - parallel world ticking (only run mid-tick at the end of each tick / fixes concurrency bugs related to executeMidTickTasks)
+ }
+ // Paper end - execute chunk tasks during tick
} // SparklyPaper end
}
}
// 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
+ java.util.ArrayList<TickingBlockEntity> oldList = new java.util.ArrayList<>(this.blockEntityTickers);
+ long diffOld = 0;
+ long diffNew = 0;
+ if (toRemoveOld.size() != 1) { // the old one always have null as the first element
+ var startOld = System.nanoTime();
+ oldList.removeAll(toRemoveOld);
+ var endOld = System.nanoTime();
+ diffOld = endOld - startOld;
+ System.out.println("Old version deleted " + toRemoveOld.size() + " elements, took " + diffOld + "ns");
+ }
+ if (startSearchFromIndex != -1) {
+ var start = System.nanoTime();
+ 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
+ var end = System.nanoTime();
+ System.out.println("(current tick: " + this.getServer().getTickCount() + ") startSearchFromIndex: " + startSearchFromIndex + " - toRemove: " + toRemove);
+ diffNew = end - start;
+ System.out.println("New version deleted " + toRemove.size() + " elements, took " + diffNew + "ns");
+ }
+ if (toRemove.size() != 0) {
+ System.out.println("Equals? " + oldList.equals(this.blockEntityTickers));
+ String winner = "Unknown";
+ long perfIncrease = 0;
+ if (diffOld > diffNew) {
+ winner = "New Version";
+ perfIncrease = diffOld - diffNew;
+ } else {
+ winner = "Old Version";
+ perfIncrease = diffNew - diffOld;
+ }
+ System.out.println("Who won? " + winner + ", by " + perfIncrease + "ns");
+ }
// SparklyPaper end
this.timings.tileEntityTick.stopTiming(); // Spigot
this.tickingBlockEntities = false;
20 changes: 6 additions & 14 deletions patches/server/0016-Optimize-tickBlockEntities.patch
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ But here's the thing: We don't care if we have a small performance penalty if th
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..df9de7f1907efd4e30eb9ab3a683761ccb4b2204 100644
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 {
Expand All @@ -28,7 +28,7 @@ index b9e0822638a3979bd43392efdb595153e6f34675..df9de7f1907efd4e30eb9ab3a683761c
protected final NeighborUpdater neighborUpdater;
private final List<TickingBlockEntity> pendingBlockEntityTickers = Lists.newArrayList();
private boolean tickingBlockEntities;
@@ -1270,15 +1270,26 @@ public abstract class Level implements LevelAccessor, AutoCloseable {
@@ -1270,8 +1270,14 @@ public abstract class Level implements LevelAccessor, AutoCloseable {
// Spigot start
// Iterator iterator = this.blockEntityTickers.iterator();
int tilesThisCycle = 0;
Expand All @@ -44,9 +44,8 @@ index b9e0822638a3979bd43392efdb595153e6f34675..df9de7f1907efd4e30eb9ab3a683761c
+ // 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);
+ TickingBlockEntity tickingblockentity = (TickingBlockEntity) this.blockEntityTickers.getWithoutBoundsChecks(this.tileTickPosition);
// Spigot start
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--;
Expand Down Expand Up @@ -182,10 +181,10 @@ index fa170cc1ce7011d201295b89718292d696c7fc24..c6f62c56da6e74fbaa57300f8cc27186
}
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..804c9fd8eedcbf701bde9b8ac0213d94fa8f70c0
index 0000000000000000000000000000000000000000..713b98c7306a8495461fa228f9f3569515bf8166
--- /dev/null
+++ b/src/main/java/net/sparklypower/sparklypaper/BlockEntityTickersList.java
@@ -0,0 +1,77 @@
@@ -0,0 +1,70 @@
+package net.sparklypower.sparklypaper;
+
+import it.unimi.dsi.fastutil.objects.ObjectArrayList;
Expand Down Expand Up @@ -220,13 +219,6 @@ index 0000000000000000000000000000000000000000..804c9fd8eedcbf701bde9b8ac0213d94
+ }
+
+ /**
+ * Gets an element by their index, without checking for array bounds.
+ */
+ public TickingBlockEntity getWithoutBoundsChecks(final int index) {
+ return a[index];
+ }
+
+ /**
+ * Removes elements by their index.
+ */
+ public boolean removeAllByIndex(final int startSearchFromIndex, final Set<Integer> c) {
Expand Down

0 comments on commit f1f4301

Please sign in to comment.