diff --git a/README.md b/README.md index 82ed643..9f8aab8 100644 --- a/README.md +++ b/README.md @@ -30,10 +30,11 @@ SparklyPaper's config file is `sparklypaper.yml`, the file is, by default, place * 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. +* ~~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.~~ + * Replaced by [Warriorrrr's "Rewrite framed map tracker ticking" patch (Paper #9605)](https://github.com/PaperMC/Paper/pull/9605) * Optimize `EntityScheduler`'s `executeTick` * On each tick, Paper runs `EntityScheduler`'s `executeTick` of each entity. This is a bit expensive, due to `ArrayDeque`'s `size()` call because it ain't a simple "get the current queue size" function, and due to the thread checks. * To avoid the hefty `ArrayDeque`'s `size()` call, we check if we *really* need to execute the `executeTick`, by checking if `currentlyExecuting` is not empty or if `oneTimeDelayed` is not empty. This brings `executeTick`'s CPU % from 2.46% down to 1.14% in a server with ~13k entities. diff --git a/patches/server/0009-Avoid-unnecessary-ItemFrame-getItem-calls.patch b/patches/removed/server/0009-Avoid-unnecessary-ItemFrame-getItem-calls.patch similarity index 100% rename from patches/server/0009-Avoid-unnecessary-ItemFrame-getItem-calls.patch rename to patches/removed/server/0009-Avoid-unnecessary-ItemFrame-getItem-calls.patch diff --git a/patches/server/0004-Rewrite-framed-map-tracker-ticking.patch b/patches/server/0004-Rewrite-framed-map-tracker-ticking.patch new file mode 100644 index 0000000..83fe117 --- /dev/null +++ b/patches/server/0004-Rewrite-framed-map-tracker-ticking.patch @@ -0,0 +1,298 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Warrior <50800980+Warriorrrr@users.noreply.github.com> +Date: Mon, 14 Aug 2023 17:34:53 +0200 +Subject: [PATCH] Rewrite framed map tracker ticking + +Rewrites the tracking code for framed maps to remove the use of the tickCarriedBy method and the HoldingPlayer class. +The tickCarriedBy method contained a lot of code that did not apply to framed maps at all, and by moving the parts +that did elsewhere, we can essentially skip it. The only logic that's ran inside the ServerEntity#sendChanges for maps +now is just updating dirty map/decoration data. + +When no bukkit renderers are added to the map, we also re-use the same packet for all players who are tracking it which avoids a lot of work. + +diff --git a/src/main/java/net/minecraft/server/level/ServerEntity.java b/src/main/java/net/minecraft/server/level/ServerEntity.java +index 196280f54e397c69d32bd4d1f6ae666efdd93773..8ef1c11969310f64d06c8421969d171b6c399ab6 100644 +--- a/src/main/java/net/minecraft/server/level/ServerEntity.java ++++ b/src/main/java/net/minecraft/server/level/ServerEntity.java +@@ -108,29 +108,42 @@ public class ServerEntity { + + Entity entity = this.entity; + +- if (!this.trackedPlayers.isEmpty() && entity instanceof ItemFrame) { // Paper - Only tick item frames if players can see it ++ if (!this.trackedPlayers.isEmpty() && entity instanceof ItemFrame frame && frame.cachedMapId != null) { // Paper - Only tick item frames if players can see it // Paper + ItemFrame entityitemframe = (ItemFrame) entity; + + if (true || this.tickCount % 10 == 0) { // CraftBukkit - Moved below, should always enter this block +- ItemStack itemstack = entityitemframe.getItem(); ++ //ItemStack itemstack = entityitemframe.getItem(); // Paper - skip redundant 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 /*&& 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 // Paper - skip redundant getItem + Integer integer = entityitemframe.cachedMapId; // Paper + MapItemSavedData worldmap = MapItem.getSavedData(integer, this.level); + + if (worldmap != null) { ++ // Paper start - re-use the same update packet when possible ++ if (!worldmap.hasContextualRenderer) { ++ // Pass in a "random" player when a non-contextual plugin renderer is added to make sure its called ++ final Packet updatePacket = worldmap.framedUpdatePacket(integer, worldmap.hasPluginRenderer ? com.google.common.collect.Iterables.getFirst(this.trackedPlayers, null).getPlayer() : null); ++ ++ if (updatePacket != null) { ++ for (ServerPlayerConnection connection : this.trackedPlayers) { ++ connection.send(updatePacket); ++ } ++ } ++ } else { ++ // Paper end + Iterator iterator = this.trackedPlayers.iterator(); // CraftBukkit + + while (iterator.hasNext()) { + ServerPlayer entityplayer = iterator.next().getPlayer(); // CraftBukkit + +- worldmap.tickCarriedBy(entityplayer, itemstack); +- Packet packet = worldmap.getUpdatePacket(integer, entityplayer); ++ //worldmap.tickCarriedBy(entityplayer, itemstack); // Paper ++ Packet packet = worldmap.framedUpdatePacket(integer, entityplayer); // Paper + + if (packet != null) { + entityplayer.connection.send(packet); + } + } ++ } // Paper + } + } + +@@ -373,6 +386,19 @@ public class ServerEntity { + } + } + ++ // Paper start - send full map when tracked ++ if (this.entity instanceof ItemFrame frame && frame.cachedMapId != null) { ++ MapItemSavedData mapData = MapItem.getSavedData(frame.cachedMapId, this.level); ++ ++ if (mapData != null) { ++ mapData.addFrameDecoration(frame); ++ ++ final Packet mapPacket = mapData.fullUpdatePacket(frame.cachedMapId, mapData.hasPluginRenderer ? player : null); ++ if (mapPacket != null) ++ sender.accept((Packet) mapPacket); ++ } ++ } ++ // Paper end + } + + private void sendDirtyEntityData() { +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..0780dd4abf035cdd4001fb9702494c54be83361a 100644 +--- a/src/main/java/net/minecraft/world/entity/decoration/ItemFrame.java ++++ b/src/main/java/net/minecraft/world/entity/decoration/ItemFrame.java +@@ -488,6 +488,16 @@ public class ItemFrame extends HangingEntity { + } + this.setItem(ItemStack.fromBukkitCopy(event.getItemStack())); + // Paper end ++ // Paper start - add decoration and mark everything dirty for other players who are already tracking this frame ++ final ItemStack item = this.getItem(); ++ if (item.is(Items.FILLED_MAP)) { ++ final MapItemSavedData data = MapItem.getSavedData(item, this.level()); ++ if (data != null) { ++ data.addFrameDecoration(this); ++ data.markAllDirty(); ++ } ++ } ++ // Paper end + this.gameEvent(GameEvent.BLOCK_CHANGE, player); + if (!player.getAbilities().instabuild) { + itemstack.shrink(1); +diff --git a/src/main/java/net/minecraft/world/level/saveddata/maps/MapItemSavedData.java b/src/main/java/net/minecraft/world/level/saveddata/maps/MapItemSavedData.java +index 50713f03c783c63f93710d986d94af544be0615a..5a0978c2342647d6293cd370e489a727c5d652ee 100644 +--- a/src/main/java/net/minecraft/world/level/saveddata/maps/MapItemSavedData.java ++++ b/src/main/java/net/minecraft/world/level/saveddata/maps/MapItemSavedData.java +@@ -66,6 +66,16 @@ public class MapItemSavedData extends SavedData { + private final Map frameMarkers = Maps.newHashMap(); + private int trackedDecorationCount; + private org.bukkit.craftbukkit.map.RenderData vanillaRender = new org.bukkit.craftbukkit.map.RenderData(); // Paper ++ // Paper start - shared between all players tracking this map inside an item frame ++ public boolean dirtyColorData; ++ public int minDirtyX; ++ public int minDirtyY; ++ public int maxDirtyX; ++ public int maxDirtyY; ++ public boolean dirtyFrameDecorations; ++ public boolean hasPluginRenderer; ++ public boolean hasContextualRenderer; ++ // Paper end + + // CraftBukkit start + public final CraftMapView mapView; +@@ -325,6 +335,7 @@ public class MapItemSavedData extends SavedData { + } + + this.setDecorationsDirty(); ++ if (mapicon != null && mapicon.renderOnFrame()) this.dirtyFrameDecorations = true; // Paper + } + + public static void addTargetDecoration(ItemStack stack, BlockPos pos, String id, MapDecoration.Type type) { +@@ -420,6 +431,7 @@ public class MapItemSavedData extends SavedData { + } + + this.setDecorationsDirty(); ++ if (type.isRenderedOnFrame() || (mapicon1 != null && mapicon.getType().isRenderedOnFrame())) this.dirtyFrameDecorations = true; // Paper + } + + } +@@ -433,6 +445,20 @@ public class MapItemSavedData extends SavedData { + + public void setColorsDirty(int x, int z) { + this.setDirty(); ++ // Paper start ++ if (this.dirtyColorData) { ++ this.minDirtyX = Math.min(this.minDirtyX, x); ++ this.minDirtyY = Math.min(this.minDirtyY, z); ++ this.maxDirtyX = Math.max(this.maxDirtyX, x); ++ this.maxDirtyY = Math.max(this.maxDirtyY, z); ++ } else { ++ this.dirtyColorData = true; ++ this.minDirtyX = x; ++ this.minDirtyY = z; ++ this.maxDirtyX = x; ++ this.maxDirtyY = z; ++ } ++ // Paper end + Iterator iterator = this.carriedBy.iterator(); + + while (iterator.hasNext()) { +@@ -515,6 +541,7 @@ public class MapItemSavedData extends SavedData { + public void removedFromFrame(BlockPos pos, int id) { + this.removeDecoration("frame-" + id); + this.frameMarkers.remove(MapFrame.frameId(pos)); ++ this.dirtyFrameDecorations = true; // Paper + } + + public boolean updateColor(int x, int z, byte color) { +@@ -572,6 +599,93 @@ public class MapItemSavedData extends SavedData { + return this.trackedDecorationCount >= iconCount; + } + ++ // Paper start ++ public final @Nullable Packet framedUpdatePacket(int id, @Nullable Player player) { ++ return createUpdatePacket(id, player, false); ++ } ++ ++ public final @Nullable Packet fullUpdatePacket(int id, @Nullable Player player) { ++ return createUpdatePacket(id, player, true); ++ } ++ ++ public final @Nullable Packet createUpdatePacket(int id, @Nullable Player player, boolean full) { ++ if (!dirtyColorData && !dirtyFrameDecorations && (player == null || server.getCurrentTick() % 5 != 0) && !full) // Periodically send update packets if a renderer is added ++ return null; ++ ++ final org.bukkit.craftbukkit.map.RenderData render = player != null ? this.mapView.render((org.bukkit.craftbukkit.entity.CraftPlayer) player.getBukkitEntity()) : this.vanillaRender; ++ ++ final MapPatch patch; ++ if (full) { ++ patch = createPatch(render.buffer, 0, 0, 127, 127); ++ } else if (dirtyColorData) { ++ dirtyColorData = false; ++ patch = createPatch(render.buffer, this.minDirtyX, this.minDirtyY, this.maxDirtyX, this.maxDirtyY); ++ } else { ++ patch = null; ++ } ++ ++ Collection decorations = null; ++ if (dirtyFrameDecorations || full || hasPluginRenderer) { // Always add decorations when a plugin renderer is added ++ dirtyFrameDecorations = false; ++ decorations = new java.util.ArrayList<>(); ++ ++ if (player == null) { ++ // We're using the vanilla renderer, add in vanilla decorations ++ for (MapDecoration decoration : this.decorations.values()) { ++ // Skip sending decorations that aren't rendered, i.e. player decorations. ++ // Skipping player decorations also allows us to send the same update packet to all tracking players, the only caveat ++ // being that it causes a slight flicker of the player decoration for anyone holding & looking at the map. ++ if (decoration.renderOnFrame()) { ++ decorations.add(decoration); ++ } ++ } ++ } ++ ++ for (final org.bukkit.map.MapCursor cursor : render.cursors) { ++ if (cursor.isVisible()) { ++ decorations.add(new MapDecoration(MapDecoration.Type.byIcon(cursor.getRawType()), cursor.getX(), cursor.getY(), cursor.getDirection(), PaperAdventure.asVanilla(cursor.caption()))); // Paper - Adventure ++ } ++ } ++ } ++ ++ return new ClientboundMapItemDataPacket(id, this.scale, this.locked, decorations, patch); ++ } ++ ++ private MapPatch createPatch(byte[] buffer, int minDirtyX, int minDirtyY, int maxDirtyX, int maxDirtyY) { ++ int i = minDirtyX; ++ int j = minDirtyY; ++ int k = maxDirtyX + 1 - minDirtyX; ++ int l = maxDirtyY + 1 - minDirtyY; ++ byte[] abyte = new byte[k * l]; ++ ++ for (int i1 = 0; i1 < k; ++i1) { ++ for (int j1 = 0; j1 < l; ++j1) { ++ abyte[i1 + j1 * k] = buffer[i + i1 + (j + j1) * 128]; ++ } ++ } ++ ++ return new MapItemSavedData.MapPatch(i, j, k, l, abyte); ++ } ++ ++ public void addFrameDecoration(net.minecraft.world.entity.decoration.ItemFrame frame) { ++ if (this.trackedDecorationCount >= frame.level().paperConfig().maps.itemFrameCursorLimit || this.frameMarkers.containsKey(MapFrame.frameId(frame.getPos()))) ++ return; ++ ++ MapFrame mapFrame = new MapFrame(frame.getPos(), frame.getDirection().get2DDataValue() * 90, frame.getId()); ++ this.addDecoration(MapDecoration.Type.FRAME, frame.level(), "frame-" + frame.getId(), frame.getPos().getX(), frame.getPos().getZ(), mapFrame.getRotation(), (Component) null); ++ this.frameMarkers.put(mapFrame.getId(), mapFrame); ++ } ++ ++ public void markAllDirty() { ++ this.dirtyColorData = true; ++ this.minDirtyX = 0; ++ this.minDirtyY = 0; ++ this.maxDirtyX = 127; ++ this.maxDirtyY = 127; ++ this.dirtyFrameDecorations = true; ++ } ++ // Paper end ++ + public class HoldingPlayer { + + // Paper start +diff --git a/src/main/java/org/bukkit/craftbukkit/map/CraftMapView.java b/src/main/java/org/bukkit/craftbukkit/map/CraftMapView.java +index 115ac54d13a8ca189a69526b1b3baf48be8a64e9..f878bed0503b183e636c15a16b766e15844f4a3f 100644 +--- a/src/main/java/org/bukkit/craftbukkit/map/CraftMapView.java ++++ b/src/main/java/org/bukkit/craftbukkit/map/CraftMapView.java +@@ -108,6 +108,10 @@ public final class CraftMapView implements MapView { + this.renderers.add(renderer); + this.canvases.put(renderer, new HashMap()); + renderer.initialize(this); ++ // Paper start ++ this.worldMap.hasPluginRenderer |= !(renderer instanceof CraftMapRenderer); ++ this.worldMap.hasContextualRenderer |= renderer.isContextual(); ++ // Paper end + } + } + +@@ -123,6 +127,17 @@ public final class CraftMapView implements MapView { + } + } + this.canvases.remove(renderer); ++ // Paper start ++ this.worldMap.hasPluginRenderer = !(this.renderers.size() == 1 && this.renderers.get(0) instanceof CraftMapRenderer); ++ if (renderer.isContextual()) { ++ // Re-check all renderers ++ boolean contextualFound = false; ++ for (final MapRenderer mapRenderer : this.renderers) { ++ contextualFound |= mapRenderer.isContextual(); ++ } ++ this.worldMap.hasContextualRenderer = contextualFound; ++ } ++ // Paper end + return true; + } else { + return false; diff --git a/patches/server/0004-Configurable-Farm-Land-moisture-tick-rate-when-the-b.patch b/patches/server/0005-Configurable-Farm-Land-moisture-tick-rate-when-the-b.patch similarity index 100% rename from patches/server/0004-Configurable-Farm-Land-moisture-tick-rate-when-the-b.patch rename to patches/server/0005-Configurable-Farm-Land-moisture-tick-rate-when-the-b.patch diff --git a/patches/server/0005-Skip-distanceToSqr-call-in-ServerEntity-sendChanges-.patch b/patches/server/0006-Skip-distanceToSqr-call-in-ServerEntity-sendChanges-.patch similarity index 100% rename from patches/server/0005-Skip-distanceToSqr-call-in-ServerEntity-sendChanges-.patch rename to patches/server/0006-Skip-distanceToSqr-call-in-ServerEntity-sendChanges-.patch diff --git a/patches/server/0006-Skip-MapItem-update-if-the-map-does-not-have-the-Cra.patch b/patches/server/0007-Skip-MapItem-update-if-the-map-does-not-have-the-Cra.patch similarity index 100% rename from patches/server/0006-Skip-MapItem-update-if-the-map-does-not-have-the-Cra.patch rename to patches/server/0007-Skip-MapItem-update-if-the-map-does-not-have-the-Cra.patch diff --git a/patches/server/0007-Fix-concurrency-issues-when-using-imageToBytes-in-mu.patch b/patches/server/0008-Fix-concurrency-issues-when-using-imageToBytes-in-mu.patch similarity index 100% rename from patches/server/0007-Fix-concurrency-issues-when-using-imageToBytes-in-mu.patch rename to patches/server/0008-Fix-concurrency-issues-when-using-imageToBytes-in-mu.patch diff --git a/patches/server/0008-Optimize-ServerStatsCounter-s-dirty-set.patch b/patches/server/0009-Optimize-ServerStatsCounter-s-dirty-set.patch similarity index 100% rename from patches/server/0008-Optimize-ServerStatsCounter-s-dirty-set.patch rename to patches/server/0009-Optimize-ServerStatsCounter-s-dirty-set.patch