From d8461e8ec526ad46de87d60ceee72a9cacdf6621 Mon Sep 17 00:00:00 2001 From: Spottedleaf Date: Fri, 30 Aug 2024 12:57:17 -0700 Subject: [PATCH] Remove toProcessTrackingUnloading This field is covered by the entity tracker optimisations in Paper, but currently was not properly maintained - resulting in memory leaks. Fixes https://github.com/PaperMC/Folia/issues/283 --- patches/server/0003-Threaded-Regions.patch | 33 ++++------------------ patches/server/0017-Region-profiler.patch | 2 +- 2 files changed, 7 insertions(+), 28 deletions(-) diff --git a/patches/server/0003-Threaded-Regions.patch b/patches/server/0003-Threaded-Regions.patch index 524df7040..6769468ca 100644 --- a/patches/server/0003-Threaded-Regions.patch +++ b/patches/server/0003-Threaded-Regions.patch @@ -2832,10 +2832,10 @@ index 0000000000000000000000000000000000000000..a1e1782d87403ca8934d37361be7ba66 +} diff --git a/src/main/java/io/papermc/paper/threadedregions/RegionizedWorldData.java b/src/main/java/io/papermc/paper/threadedregions/RegionizedWorldData.java new file mode 100644 -index 0000000000000000000000000000000000000000..5dc4c8a5b896f519be5414d4a53a71144c225324 +index 0000000000000000000000000000000000000000..cd89a11ff94dfbb9c9e259b85f931bd0d160c06e --- /dev/null +++ b/src/main/java/io/papermc/paper/threadedregions/RegionizedWorldData.java -@@ -0,0 +1,772 @@ +@@ -0,0 +1,751 @@ +package io.papermc.paper.threadedregions; + +import ca.spottedleaf.moonrise.common.list.IteratorSafeOrderedReferenceSet; @@ -2925,9 +2925,6 @@ index 0000000000000000000000000000000000000000..5dc4c8a5b896f519be5414d4a53a7114 + for (final Entity entity : from.loadedEntities) { + into.loadedEntities.add(entity); + } -+ for (final Entity entity : from.toProcessTrackingUnloading) { -+ into.toProcessTrackingUnloading.add(entity); -+ } + for (final Iterator iterator = from.entityTickList.unsafeIterator(); iterator.hasNext();) { + into.entityTickList.add(iterator.next()); + } @@ -3029,11 +3026,6 @@ index 0000000000000000000000000000000000000000..5dc4c8a5b896f519be5414d4a53a7114 + if (from.loadedEntities.contains(entity)) { + into.loadedEntities.add(entity); + } -+ // Note: toProcessTrackingUnloading is not a subset of allEntities, but for the cases where it is not -+ // do not matter as the tracker removal is handled -+ if (from.toProcessTrackingUnloading.contains(entity)) { -+ into.toProcessTrackingUnloading.add(entity); -+ } + // Note: navigatingMobs is a subset of allEntities + if (entity instanceof Mob mob && from.navigatingMobs.contains(mob)) { + into.navigatingMobs.add(mob); @@ -3178,7 +3170,6 @@ index 0000000000000000000000000000000000000000..5dc4c8a5b896f519be5414d4a53a7114 + private final NearbyPlayers nearbyPlayers; + private final ReferenceList allEntities = new ReferenceList<>(EMPTY_ENTITY_ARRAY); + private final ReferenceList loadedEntities = new ReferenceList<>(EMPTY_ENTITY_ARRAY); -+ private final ReferenceList toProcessTrackingUnloading = new ReferenceList<>(EMPTY_ENTITY_ARRAY); + private final IteratorSafeOrderedReferenceSet entityTickList = new IteratorSafeOrderedReferenceSet<>(); + private final IteratorSafeOrderedReferenceSet navigatingMobs = new IteratorSafeOrderedReferenceSet<>(); + public final ReferenceList trackerEntities = new ReferenceList<>(EMPTY_ENTITY_ARRAY); // Moonrise - entity tracker @@ -3386,9 +3377,7 @@ index 0000000000000000000000000000000000000000..5dc4c8a5b896f519be5414d4a53a7114 + } + + public void addLoadedEntity(final Entity entity) { -+ if (this.loadedEntities.add(entity)) { -+ this.toProcessTrackingUnloading.remove(entity); -+ } ++ this.loadedEntities.add(entity); + } + + public boolean hasLoadedEntity(final Entity entity) { @@ -3396,23 +3385,13 @@ index 0000000000000000000000000000000000000000..5dc4c8a5b896f519be5414d4a53a7114 + } + + public void removeLoadedEntity(final Entity entity) { -+ if (this.loadedEntities.remove(entity)) { -+ this.toProcessTrackingUnloading.add(entity); -+ } ++ this.loadedEntities.remove(entity); + } + + public Iterable getLoadedEntities() { + return this.loadedEntities; + } + -+ public Entity[] takeTrackingUnloads() { -+ final Entity[] ret = Arrays.copyOf(this.toProcessTrackingUnloading.getRawData(), this.toProcessTrackingUnloading.size(), Entity[].class); -+ -+ this.toProcessTrackingUnloading.clear(); -+ -+ return ret; -+ } -+ + public void addEntityTickingEntity(final Entity entity) { + if (!TickThread.isTickThreadFor(entity)) { + throw new IllegalArgumentException("Entity " + entity + " is not under this region's control"); @@ -10327,7 +10306,7 @@ index dd56c8e041116ef3602a9f89c998c8208ab89b51..301d7fa29fce2997a5881b3852896eff if (waitableArray[0] != null) { //noinspection unchecked diff --git a/src/main/java/net/minecraft/server/level/ChunkMap.java b/src/main/java/net/minecraft/server/level/ChunkMap.java -index 1e0a6e5a3c907ab55ee6f2780a7d43bd455f2b7b..7f161864ee6a43de8d37c0e5c9cba9918f2fed89 100644 +index 1e0a6e5a3c907ab55ee6f2780a7d43bd455f2b7b..dff1f17a4c164e82ed4f5d9e9f48ee62c671e589 100644 --- a/src/main/java/net/minecraft/server/level/ChunkMap.java +++ b/src/main/java/net/minecraft/server/level/ChunkMap.java @@ -138,8 +138,8 @@ public class ChunkMap extends ChunkStorage implements ChunkHolder.PlayerProvider @@ -10478,7 +10457,7 @@ index 1e0a6e5a3c907ab55ee6f2780a7d43bd455f2b7b..7f161864ee6a43de8d37c0e5c9cba991 - if (playerchunkmap_entitytracker1.entity != entityplayer) { - playerchunkmap_entitytracker1.updatePlayer(entityplayer); + // Folia start - region threading -+ for (Entity possible : this.level.getCurrentWorldData().getLoadedEntities()) { ++ for (Entity possible : this.level.getCurrentWorldData().trackerEntities) { + if (possible.moonrise$getTrackedEntity() != null) { + possible.moonrise$getTrackedEntity().updatePlayer(entityplayer); } diff --git a/patches/server/0017-Region-profiler.patch b/patches/server/0017-Region-profiler.patch index 7e884e27f..8db3b3c6b 100644 --- a/patches/server/0017-Region-profiler.patch +++ b/patches/server/0017-Region-profiler.patch @@ -1460,7 +1460,7 @@ index eda5f0d099f9f8621de8ad7808098abf6f5cb544..ac56b02498eb38883ae462be6ef3d15c this.profiler.popPush("players"); MinecraftTimings.playerListTimer.startTiming(); // Spigot // Paper diff --git a/src/main/java/net/minecraft/server/level/ChunkMap.java b/src/main/java/net/minecraft/server/level/ChunkMap.java -index 7f161864ee6a43de8d37c0e5c9cba9918f2fed89..31f2f6e841179a2fba2199afa7d6211eb68d3c07 100644 +index dff1f17a4c164e82ed4f5d9e9f48ee62c671e589..728454fcfbeddcfe1b0a95e89827f33d7f7838bc 100644 --- a/src/main/java/net/minecraft/server/level/ChunkMap.java +++ b/src/main/java/net/minecraft/server/level/ChunkMap.java @@ -392,13 +392,18 @@ public class ChunkMap extends ChunkStorage implements ChunkHolder.PlayerProvider