Skip to content

Commit

Permalink
Refactor MapItem update skip, add synchornized to CraftMapColorCache'…
Browse files Browse the repository at this point in the history
…s "isCached" function
  • Loading branch information
MrPowerGamerBR committed Nov 17, 2023
1 parent d0b69c2 commit 4098309
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 15 deletions.
6 changes: 4 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@ SparklyPaper's config file is `sparklypaper.yml`, the file is, by default, place
* Skip `distanceToSqr` call in `ServerEntity#sendChanges` if the delta movement hasn't changed
* The `distanceToSqr` call is a bit expensive, so avoiding it is pretty nice, around ~15% calls are skipped with this check. Currently, we only check if both Vec3 objects have the same identity, that means, if they are literally the same object. (that works because Minecraft's code reuses the Vec3 object when caching the current delta movement)
* Skip `MapItem#update()` if the map does not have the CraftMapRenderer present
* By default, custom maps, even those with custom renderers, are still fetching world data to update the map data. With this change, "image in map" maps can avoid these hefty updates, without requiring the map to be locked, which some old map plugins may not do.
* This has the disadvantage that the vanilla map data will never be updated while the CraftMapRenderer is not present, so if you readd the default renderer, the server will need to update the map data, but that's not a huuuge problem for us.
* By default, maps, even those with custom renderers, fetch the world data to update the map data. With this change, "image in map" maps can avoid these hefty updates, without requiring the map to be locked, which some old map plugins may not do.
* This has the disadvantage that the vanilla map data will never be updated while the CraftMapRenderer is not present, so if you readd the default renderer, the server will need to update the map data, but that's not a huuuge problem, after all, it is a very rare circumstance that you may need the map data to always be up-to-date when you have a custom renderer on the map.
* SparklyPaper - fix concurrency issues when using `imageToBytes` in multiple threads
* Useful if one of your plugins is parallelizng map creation on server startup
* 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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,22 +9,15 @@ Optimizes "image in map" maps, without requiring the map to be locked, which som
This has the disadvantage that the vanilla map data will never be updated while the CraftMapRenderer is not present, but that's not a huuuge problem for us

diff --git a/src/main/java/net/minecraft/world/item/MapItem.java b/src/main/java/net/minecraft/world/item/MapItem.java
index c368b437597edf7e165326727ae778a69c3fcc83..c54498c662fef50ffb16441e1480f4e048ff99d6 100644
index c368b437597edf7e165326727ae778a69c3fcc83..190716ea7857c7ad5427a527ad8d6b5a241a00b6 100644
--- a/src/main/java/net/minecraft/world/item/MapItem.java
+++ b/src/main/java/net/minecraft/world/item/MapItem.java
@@ -333,7 +333,14 @@ public class MapItem extends ComplexItem {
@@ -332,7 +332,7 @@ public class MapItem extends ComplexItem {
worldmap.tickCarriedBy(entityhuman, stack);
}

if (!worldmap.locked && (selected || entity instanceof Player && ((Player) entity).getOffhandItem() == stack)) {
- this.update(world, entity, worldmap);
+ // SparklyPaper - don't update maps if they don't have the CraftMapRenderer in the render list
+ if (!world.sparklyPaperConfig.getSkipMapItemDataUpdatesIfMapDoesNotHaveCraftMapRenderer() || worldmap.mapView.getRenderers().stream().anyMatch(mapRenderer -> mapRenderer.getClass() == org.bukkit.craftbukkit.map.CraftMapRenderer.class)) {
+ this.update(world, entity, worldmap);
+ } else {
+ if (entity instanceof Player)
+ worldmap.getHoldingPlayer((Player) entity); // Required to add the player to the carriedBy list, which causes a map update
+ }
+ // SparklyPaper end
- if (!worldmap.locked && (selected || entity instanceof Player && ((Player) entity).getOffhandItem() == stack)) {
+ if (!worldmap.locked && (!world.sparklyPaperConfig.getSkipMapItemDataUpdatesIfMapDoesNotHaveCraftMapRenderer() || worldmap.mapView.getRenderers().stream().anyMatch(mapRenderer -> mapRenderer.getClass() == org.bukkit.craftbukkit.map.CraftMapRenderer.class)) && (selected || entity instanceof Player && ((Player) entity).getOffhandItem() == stack)) { // SparklyPaper - don't update maps if they don't have the CraftMapRenderer in the render list
this.update(world, entity, worldmap);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: MrPowerGamerBR <git@mrpowergamerbr.com>
Date: Fri, 17 Nov 2023 19:08:08 -0300
Subject: [PATCH] Fix concurrency issues when using "imageToBytes" in multiple
threads

Useful if one of your plugins is parallelizng map creation on server startup

diff --git a/src/main/java/org/bukkit/craftbukkit/map/CraftMapColorCache.java b/src/main/java/org/bukkit/craftbukkit/map/CraftMapColorCache.java
index 8149b9c51b78eb5c689b7218a2ca3aab60e73bcf..c983d8d7e79d55c9757add8ac1093a0a9d98e5b3 100644
--- a/src/main/java/org/bukkit/craftbukkit/map/CraftMapColorCache.java
+++ b/src/main/java/org/bukkit/craftbukkit/map/CraftMapColorCache.java
@@ -145,7 +145,7 @@ public class CraftMapColorCache implements MapPalette.MapColorCache {
}

@Override
- public boolean isCached() {
+ public synchronized boolean isCached() { // SparklyPaper - fix concurrency issues when using "imageToBytes" in multiple threads
return this.cached || (!this.running.get() && this.initCache().isDone());
}

0 comments on commit 4098309

Please sign in to comment.