Fix 2GB/sec memory allocation on pathfinders #425

- WeakReference on PathfindExecutors
- OPTIMIZE Instance Creation on PathfinderExecutorExecutor, it was previously allocating 2GB/sec (lol)
- Cleanup reference to Executor for it to be garbage collected
- Volatile on target on executor, sometimes it was not updating
- Fix pathfind stopping if visited all open nodes
- Fix isBlocked array not correctly being updated due to multi-threading


Signed-off-by: syeyoung <cyoung06@naver.com>

11 files changed, 92 insertions, 36 deletions

diff --git a/mod/src/main/java/kr/syeyoung/dungeonsguide/mod/dungeon/DungeonContext.java b/mod/src/main/java/kr/syeyoung/dungeonsguide/mod/dungeon/DungeonContext.java
index c1d4cff3..acafd33e 100755
--- a/mod/src/main/java/kr/syeyoung/dungeonsguide/mod/dungeon/DungeonContext.java
+++ b/mod/src/main/java/kr/syeyoung/dungeonsguide/mod/dungeon/DungeonContext.java
@@ -51,6 +51,7 @@ import net.minecraftforge.common.MinecraftForge;
 
 import javax.vecmath.Vector2d;
 import java.awt.*;
+import java.lang.ref.WeakReference;
 import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.List;
@@ -70,7 +71,7 @@ public class DungeonContext {
     private DungeonEventRecorder recorder = new DungeonEventRecorder();
 
     @Getter
-    private List<PathfinderExecutor> executors = new CopyOnWriteArrayList<>();
+    private List<WeakReference<PathfinderExecutor>> executors = new CopyOnWriteArrayList<>();
 
 
     @Getter
@@ -165,8 +166,10 @@ public class DungeonContext {
                     dungeonRoom.tryRematch();
                 }
             }
+            executor.setRoomIn(scaffoldParser.getRoomMap().get(getScaffoldParser().getDungeonMapLayout().worldPointToRoomPoint(Minecraft.getMinecraft().thePlayer.getPosition())));
         }
 
+
         players.clear();
         players.addAll(TabListUtil.getPlayersInDungeon());
     }
diff --git a/mod/src/main/java/kr/syeyoung/dungeonsguide/mod/dungeon/PathfinderExecutorExecutor.java b/mod/src/main/java/kr/syeyoung/dungeonsguide/mod/dungeon/PathfinderExecutorExecutor.java
index 676dd017..fc991023 100644
--- a/mod/src/main/java/kr/syeyoung/dungeonsguide/mod/dungeon/PathfinderExecutorExecutor.java
+++ b/mod/src/main/java/kr/syeyoung/dungeonsguide/mod/dungeon/PathfinderExecutorExecutor.java
@@ -22,8 +22,13 @@ import kr.syeyoung.dungeonsguide.mod.DungeonsGuide;
 import kr.syeyoung.dungeonsguide.mod.dungeon.pathfinding.algorithms.PathfinderExecutor;
 import kr.syeyoung.dungeonsguide.mod.dungeon.roomfinder.DungeonRoom;
 import net.minecraft.client.Minecraft;
+import net.minecraft.util.BlockPos;
+import scala.reflect.internal.util.WeakHashSet;
 
 import java.awt.*;
+import java.lang.ref.WeakReference;
+import java.util.ArrayList;
+import java.util.List;
 
 public class PathfinderExecutorExecutor extends Thread{
     public PathfinderExecutorExecutor(DungeonContext context) {
@@ -31,18 +36,39 @@ public class PathfinderExecutorExecutor extends Thread{
         this.context =context;
     }
     private DungeonContext context;
+    private DungeonRoom target;
+
+    public void setRoomIn(DungeonRoom target) {
+        this.target = target;
+    }
+
     @Override
     public void run() {
+        List<WeakReference<PathfinderExecutor>> toRemove = new ArrayList<>();
+        WeakReference<PathfinderExecutor>[] weakReferences = new WeakReference[200]; // shoulllld be enough
         while(!isInterrupted()) {
             if (context.getScaffoldParser() != null) {
-                Point roomPt = context.getScaffoldParser().getDungeonMapLayout().worldPointToRoomPoint(Minecraft.getMinecraft().thePlayer.getPosition());
-
-                DungeonRoom dungeonRoom = context.getScaffoldParser().getRoomMap().get(roomPt);
                 try {
-                    for (PathfinderExecutor executor : context.getExecutors()) {
-                        if (executor.getDungeonRoom() == dungeonRoom)
-                            executor.doStep();
+                    boolean flag = false;
+                    context.getExecutors().toArray(weakReferences);
+                    for (int i = 0; i < weakReferences.length; i++) {
+                        WeakReference<PathfinderExecutor> executor = weakReferences[i];
+                        if (executor == null) break;
+
+                        PathfinderExecutor executor1 = executor.get();
+                        if (executor1 != null) {
+                            if (executor1.getDungeonRoom() == target)
+                                executor1.doStep();
+                        } else {
+                            flag = true;
+                            toRemove.add(executor);
+                        }
+                    }
+                    if (flag) {
+                        context.getExecutors().removeAll(toRemove);
+                        toRemove.clear();
                     }
+//                    Thread.yield();
                 } catch (Exception e) {
                     e.printStackTrace(); // wtf?
                     try {
diff --git a/mod/src/main/java/kr/syeyoung/dungeonsguide/mod/dungeon/actions/AbstractAction.java b/mod/src/main/java/kr/syeyoung/dungeonsguide/mod/dungeon/actions/AbstractAction.java
index 42f43bb9..80b98a4f 100644
--- a/mod/src/main/java/kr/syeyoung/dungeonsguide/mod/dungeon/actions/AbstractAction.java
+++ b/mod/src/main/java/kr/syeyoung/dungeonsguide/mod/dungeon/actions/AbstractAction.java
@@ -51,6 +51,10 @@ public abstract class AbstractAction {
 
     }
 
+    public void cleanup(DungeonRoom dungeonRoom, ActionRouteProperties actionRouteProperties) {
+
+    }
+
     public Set<AbstractAction> getPreRequisites(DungeonRoom dungeonRoom) {
         return null;
     }
diff --git a/mod/src/main/java/kr/syeyoung/dungeonsguide/mod/dungeon/actions/ActionMove.java b/mod/src/main/java/kr/syeyoung/dungeonsguide/mod/dungeon/actions/ActionMove.java
index 9ccbee8c..dd0cd248 100755
--- a/mod/src/main/java/kr/syeyoung/dungeonsguide/mod/dungeon/actions/ActionMove.java
+++ b/mod/src/main/java/kr/syeyoung/dungeonsguide/mod/dungeon/actions/ActionMove.java
@@ -105,6 +105,10 @@ public class ActionMove extends AbstractAction {
         }
     }
 
+    @Override
+    public void cleanup(DungeonRoom dungeonRoom, ActionRouteProperties actionRouteProperties) {
+        executor = null;
+    }
 
     public void forceRefresh(DungeonRoom dungeonRoom) {
         if (executor == null) executor = dungeonRoom.createEntityPathTo(target.getBlockPos(dungeonRoom));
diff --git a/mod/src/main/java/kr/syeyoung/dungeonsguide/mod/dungeon/actions/ActionMoveNearestAir.java b/mod/src/main/java/kr/syeyoung/dungeonsguide/mod/dungeon/actions/ActionMoveNearestAir.java
index e832a59a..d4e40ad9 100755
--- a/mod/src/main/java/kr/syeyoung/dungeonsguide/mod/dungeon/actions/ActionMoveNearestAir.java
+++ b/mod/src/main/java/kr/syeyoung/dungeonsguide/mod/dungeon/actions/ActionMoveNearestAir.java
@@ -78,6 +78,11 @@ public class ActionMoveNearestAir extends AbstractAction {
         }
     }
 
+    @Override
+    public void cleanup(DungeonRoom dungeonRoom, ActionRouteProperties actionRouteProperties) {
+        executor = null;
+    }
+
     public void forceRefresh(DungeonRoom dungeonRoom) {
         if (executor == null) executor = dungeonRoom.createEntityPathTo(target.getBlockPos(dungeonRoom));
         executor.setTarget(Minecraft.getMinecraft().thePlayer.getPositionVector());
diff --git a/mod/src/main/java/kr/syeyoung/dungeonsguide/mod/dungeon/actions/tree/ActionRoute.java b/mod/src/main/java/kr/syeyoung/dungeonsguide/mod/dungeon/actions/tree/ActionRoute.java
index 428df236..de4af85a 100644
--- a/mod/src/main/java/kr/syeyoung/dungeonsguide/mod/dungeon/actions/tree/ActionRoute.java
+++ b/mod/src/main/java/kr/syeyoung/dungeonsguide/mod/dungeon/actions/tree/ActionRoute.java
@@ -60,6 +60,10 @@ public class ActionRoute {
     }
 
     public AbstractAction next() {
+        if (!(getCurrentAction() instanceof  ActionMove || getCurrentAction() instanceof  ActionMoveNearestAir))
+            getCurrentAction().cleanup(dungeonRoom, actionRouteProperties);
+        if (this.current -1 >= 0 && (actions.get(this.current-1) instanceof ActionMove || actions.get(this.current-1) instanceof ActionMoveNearestAir))
+            actions.get(this.current-1).cleanup(dungeonRoom, actionRouteProperties);
         current ++;
         if (current >= actions.size()) {
             current = actions.size() - 1;
diff --git a/mod/src/main/java/kr/syeyoung/dungeonsguide/mod/dungeon/pathfinding/algorithms/AStarCornerCut.java b/mod/src/main/java/kr/syeyoung/dungeonsguide/mod/dungeon/pathfinding/algorithms/AStarCornerCut.java
index 08e88392..8a8f5e22 100644
--- a/mod/src/main/java/kr/syeyoung/dungeonsguide/mod/dungeon/pathfinding/algorithms/AStarCornerCut.java
+++ b/mod/src/main/java/kr/syeyoung/dungeonsguide/mod/dungeon/pathfinding/algorithms/AStarCornerCut.java
@@ -74,7 +74,7 @@ public class AStarCornerCut  implements IPathfinder {
     public boolean doOneStep() {
         if (found) return true;
         Node n = open.poll();
-        if (n == null) return false;
+        if (n == null) return true;
         if (n.lastVisited == pfindIdx) return false;
         n.lastVisited = pfindIdx;
 
diff --git a/mod/src/main/java/kr/syeyoung/dungeonsguide/mod/dungeon/pathfinding/algorithms/AStarFineGrid.java b/mod/src/main/java/kr/syeyoung/dungeonsguide/mod/dungeon/pathfinding/algorithms/AStarFineGrid.java
index 3811076b..f492afc4 100644
--- a/mod/src/main/java/kr/syeyoung/dungeonsguide/mod/dungeon/pathfinding/algorithms/AStarFineGrid.java
+++ b/mod/src/main/java/kr/syeyoung/dungeonsguide/mod/dungeon/pathfinding/algorithms/AStarFineGrid.java
@@ -73,7 +73,7 @@ public class AStarFineGrid implements IPathfinder {
     public boolean doOneStep() {
         if (found) return true;
         Node n = open.poll();
-        if (n == null) return false;
+        if (n == null) return true;
         if (n.lastVisited == pfindIdx) return false;
         n.lastVisited = pfindIdx;
 
diff --git a/mod/src/main/java/kr/syeyoung/dungeonsguide/mod/dungeon/pathfinding/algorithms/PathfinderExecutor.java b/mod/src/main/java/kr/syeyoung/dungeonsguide/mod/dungeon/pathfinding/algorithms/PathfinderExecutor.java
index 3c433237..704a4f9d 100644
--- a/mod/src/main/java/kr/syeyoung/dungeonsguide/mod/dungeon/pathfinding/algorithms/PathfinderExecutor.java
+++ b/mod/src/main/java/kr/syeyoung/dungeonsguide/mod/dungeon/pathfinding/algorithms/PathfinderExecutor.java
@@ -28,7 +28,7 @@ import java.util.List;
 public class PathfinderExecutor {
     private boolean invalidate = false;
     @Getter
-    private Vec3 target;
+    private volatile Vec3 target;
 
     @Getter
     private DungeonRoom dungeonRoom;
diff --git a/mod/src/main/java/kr/syeyoung/dungeonsguide/mod/dungeon/pathfinding/algorithms/ThetaStar.java b/mod/src/main/java/kr/syeyoung/dungeonsguide/mod/dungeon/pathfinding/algorithms/ThetaStar.java
index 0916af59..b5891069 100644
--- a/mod/src/main/java/kr/syeyoung/dungeonsguide/mod/dungeon/pathfinding/algorithms/ThetaStar.java
+++ b/mod/src/main/java/kr/syeyoung/dungeonsguide/mod/dungeon/pathfinding/algorithms/ThetaStar.java
@@ -73,7 +73,7 @@ public class ThetaStar implements IPathfinder {
     public boolean doOneStep() {
         if (found) return true;
         Node n = open.poll();
-        if (n == null) return false;
+        if (n == null) return true; // nothing more to search. this means no route.
         if (n.lastVisited == pfindIdx) return false;
         n.lastVisited = pfindIdx;
 
diff --git a/mod/src/main/java/kr/syeyoung/dungeonsguide/mod/dungeon/roomfinder/DungeonRoom.java b/mod/src/main/java/kr/syeyoung/dungeonsguide/mod/dungeon/roomfinder/DungeonRoom.java
index 998f8a1e..ea1ce09b 100755
--- a/mod/src/main/java/kr/syeyoung/dungeonsguide/mod/dungeon/roomfinder/DungeonRoom.java
+++ b/mod/src/main/java/kr/syeyoung/dungeonsguide/mod/dungeon/roomfinder/DungeonRoom.java
@@ -57,6 +57,7 @@ import net.minecraft.world.chunk.storage.ExtendedBlockStorage;
 
 import javax.vecmath.Vector2d;
 import java.awt.*;
+import java.lang.ref.WeakReference;
 import java.util.List;
 import java.util.*;
 import java.util.concurrent.*;
@@ -136,11 +137,17 @@ public class DungeonRoom {
         this.currentState = currentState;
     }
 
-    private final Map<BlockPos, PathfinderExecutor> activePathfind = new HashMap<>();
+    private final Map<BlockPos, WeakReference<PathfinderExecutor>> activePathfind = new HashMap<>();
 
     public PathfinderExecutor createEntityPathTo(BlockPos pos) {
         FeaturePathfindStrategy.PathfindStrategy pathfindStrategy = FeatureRegistry.SECRET_PATHFIND_STRATEGY.getPathfindStrat();
-        if (activePathfind.containsKey(pos)) return  activePathfind.get(pos);
+        if (activePathfind.containsKey(pos)) {
+            WeakReference<PathfinderExecutor> executorWeakReference = activePathfind.get(pos);
+            PathfinderExecutor executor = executorWeakReference.get();
+            if (executor != null) {
+                return executor;
+            }
+        }
         PathfinderExecutor executor;
         if (pathfindStrategy == FeaturePathfindStrategy.PathfindStrategy.A_STAR_FINE_GRID) {
             executor = new PathfinderExecutor(new AStarFineGrid(), new Vec3(pos.getX(), pos.getY(), pos.getZ()).addVector(0.5, 0.5, 0.5), this);
@@ -151,8 +158,8 @@ public class DungeonRoom {
         } else {
             return  null;
         }
-        activePathfind.put(pos, executor);
-        context.getExecutors().add(executor);
+        activePathfind.put(pos, new WeakReference<>(executor));
+        context.getExecutors().add(new WeakReference<>(executor));
         return executor;
     }
     private static final ExecutorService roomMatcherThread = DungeonsGuide.getDungeonsGuide().registerExecutorService(Executors.newSingleThreadExecutor(
@@ -295,8 +302,8 @@ public class DungeonRoom {
     }
 
     public BlockPos getRelativeBlockPosAt(int x, int y, int z) {
-            BlockPos pos = new BlockPos(x,y,z).add(min.getX(),min.getY(),min.getZ());
-            return pos;
+        BlockPos pos = new BlockPos(x,y,z).add(min.getX(),min.getY(),min.getZ());
+        return pos;
     }
 
     public int getRelativeBlockDataAt(int x, int y, int z) {
@@ -342,14 +349,9 @@ public class DungeonRoom {
     private final int maxz;
     private final int lenx, leny, lenz;
     private static final float playerWidth = 0.3f;
-    public boolean isBlocked(int x,int y, int z) {
-        if (x < minx || z < minz || x >= maxx || z >= maxz || y < miny || y >= maxy) return true;
-        int dx = x - minx, dy = y - miny, dz = z - minz;
-        int bitIdx = dx * leny * lenz + dy * lenz + dz;
-        int location = bitIdx / 4;
-        int bitStart = (2 * (bitIdx % 4));
-        long theBit = arr[location];
-        if (((theBit >> bitStart) & 0x2) > 0) return ((theBit >> bitStart) & 1) > 0;
+
+    private int calculateIsBlocked(int x, int y, int z) {
+        if (x < minx || z < minz || x >= maxx || z >= maxz || y < miny || y >= maxy) return 3;
         float wX = x / 2.0f, wY = y / 2.0f, wZ = z / 2.0f;
 
 
@@ -374,30 +376,38 @@ public class DungeonRoom {
                     if (b.isFullCube() && i2 == k-1) continue;
                     if (iBlockState1.equals( NodeProcessorDungeonRoom.preBuilt)) continue;
                     if (b.isFullCube()) {
-                        theBit |= (3L << bitStart);
-                        arr[location] = theBit;
-                        return true;
+                        return 3;
                     }
                     try {
                         b.addCollisionBoxesToList(getCachedWorld(), blockPos, iBlockState1, bb, list, null);
                     } catch (Exception e) {
-                        return true;
+                        return 3;
                     }
                     if (list.size() > 0) {
-                        theBit |= (3L << bitStart);
-                        arr[location] = theBit;
-                        return true;
+                        return 3;
                     }
                 }
             }
         }
-        theBit |= 2L << bitStart;
+        return 2;
+    }
+    public boolean isBlocked(int x,int y, int z) {
+        if (x < minx || z < minz || x >= maxx || z >= maxz || y < miny || y >= maxy) return true;
+        int dx = x - minx, dy = y - miny, dz = z - minz;
+        int bitIdx = dx * leny * lenz + dy * lenz + dz;
+        int location = bitIdx / 4;
+        int bitStart = (2 * (bitIdx % 4));
+        long theBit = arr[location];
+        if (((theBit >> bitStart) & 0x2) > 0) return ((theBit >> bitStart) & 1) > 0;
+
+        long val = calculateIsBlocked(x, y, z);
+        theBit |= val << bitStart;
         arr[location] = theBit;
-        return false;
+        return val == 3;
     }
 
 
-    public void resetBlock(BlockPos pos) {
+    public void resetBlock(BlockPos pos) { // I think it can be optimize due to how it is saved in arr
         for (int x = -1; x <= 1; x++) {
             for (int y = -1; y <= 1; y++) {
                 for (int z = -1; z <= 1; z++) {
@@ -411,6 +421,6 @@ public class DungeonRoom {
         int dx = x - minx, dy = y - miny, dz = z - minz;
         int bitIdx = dx * leny * lenz + dy * lenz + dz;
         int location = bitIdx / 4;
-        arr[location] = 0;
+        arr[location] = (arr[location] & ~(3L << (2 * (bitIdx % 4)))) | (long) calculateIsBlocked(x, y, z) << (2 * (bitIdx % 4));
     }
 }