Fixed PerGroupChunkProvider

src/main/java/net/minestom/server/UpdateManager.java

@@ -8,7 +8,7 @@ import net.minestom.server.instance.Instance;
 import net.minestom.server.instance.InstanceManager;
 import net.minestom.server.network.ConnectionManager;
 import net.minestom.server.network.packet.server.play.KeepAlivePacket;
-import net.minestom.server.thread.PerInstanceThreadProvider;
+import net.minestom.server.thread.PerGroupChunkProvider;
 import net.minestom.server.thread.ThreadProvider;
 import net.minestom.server.utils.thread.MinestomThread;
 import net.minestom.server.utils.validate.Check;
@@ -26,8 +26,8 @@ public final class UpdateManager {
     private ThreadProvider threadProvider;
 
     {
-        threadProvider = new PerInstanceThreadProvider();
+        //threadProvider = new PerInstanceThreadProvider();
-        //threadProvider = new PerGroupChunkProvider();
+        threadProvider = new PerGroupChunkProvider();
     }
 
     /**
@@ -108,7 +108,7 @@ public final class UpdateManager {
      * @param chunkX   the chunk X
      * @param chunkZ   the chunk Z
      */
-    public void signalChunkLoad(Instance instance, int chunkX, int chunkZ) {
+    public synchronized void signalChunkLoad(Instance instance, int chunkX, int chunkZ) {
         if (this.threadProvider == null)
             return;
         this.threadProvider.onChunkLoad(instance, chunkX, chunkZ);
@@ -121,7 +121,7 @@ public final class UpdateManager {
      * @param chunkX   the chunk X
      * @param chunkZ   the chunk Z
      */
-    public void signalChunkUnload(Instance instance, int chunkX, int chunkZ) {
+    public synchronized void signalChunkUnload(Instance instance, int chunkX, int chunkZ) {
         if (this.threadProvider == null)
             return;
         this.threadProvider.onChunkUnload(instance, chunkX, chunkZ);

src/main/java/net/minestom/server/entity/EntityCreature.java

@@ -67,6 +67,10 @@ public abstract class EntityCreature extends LivingEntity {
     @Override
     public void update(long time) {
 
+        if (getInstance() == null) {
+            return;
+        }
+
         {
             // Supplier used to get the next goal selector which should start
             // (null if not found)
@@ -209,8 +213,6 @@ public abstract class EntityCreature extends LivingEntity {
     @Override
     public boolean addViewer(Player player) {
         final boolean result = super.addViewer(player);
-        if (!result)
-            return false;
 
         PlayerConnection playerConnection = player.getPlayerConnection();
 

src/main/java/net/minestom/server/thread/PerGroupChunkProvider.java

@@ -4,92 +4,135 @@ import net.minestom.server.instance.Chunk;
 import net.minestom.server.instance.Instance;
 import net.minestom.server.utils.chunk.ChunkUtils;
 
-import java.util.HashMap;
+import java.util.*;
-import java.util.HashSet;
-import java.util.Map;
-import java.util.Set;
 
 /**
  * Separate chunks into group of linked chunks
  * <p>
  * (1 chunks group = 1 thread execution)
  */
-// TODO
 public class PerGroupChunkProvider extends ThreadProvider {
 
     /**
-     * Here are stored all cached chunks waiting for a ChunkGroup
+     * Chunk -> its chunk group
      */
-    private Map<Chunk, Set<ChunkCoordinate>> cachedChunks = new HashMap<>();
+    private Map<Instance, Map<ChunkCoordinate, Set<ChunkCoordinate>>> instanceChunksGroupMap = new HashMap<>();
 
     /**
      * Used to know to which instance is linked a Set of chunks
      */
-    private Map<Set<ChunkCoordinate>, Instance> instanceMap = new HashMap<>();
+    private Map<Instance, Map<Set<ChunkCoordinate>, Instance>> instanceInstanceMap = new HashMap<>();
 
     @Override
     public void onChunkLoad(Instance instance, int chunkX, int chunkZ) {
 
+        Map<ChunkCoordinate, Set<ChunkCoordinate>> chunksGroupMap = getChunksGroupMap(instance);
+        Map<Set<ChunkCoordinate>, Instance> instanceMap = getInstanceMap(instance);
+
+        // List of groups which are neighbours
+        List<Set<ChunkCoordinate>> neighboursGroups = new ArrayList<>();
+
+        final List<ChunkCoordinate> chunks = getNeighbours(instance, chunkX, chunkZ);
+        boolean findGroup = false;
+        for (ChunkCoordinate chunkCoordinate : chunks) {
+            if (chunksGroupMap.containsKey(chunkCoordinate)) {
+                final Set<ChunkCoordinate> group = chunksGroupMap.get(chunkCoordinate);
+                neighboursGroups.add(group);
+                chunksGroupMap.remove(chunkCoordinate);
+                instanceMap.remove(group);
+                findGroup = true;
+            }
+        }
+
+        if (!findGroup) {
+            // Create group of one chunk
+            final ChunkCoordinate chunkCoordinate = new ChunkCoordinate(chunkX, chunkZ);
+            Set<ChunkCoordinate> chunkCoordinates = new HashSet<>();
+            chunkCoordinates.add(chunkCoordinate);
+
+            chunksGroupMap.put(chunkCoordinate, chunkCoordinates);
+            instanceMap.put(chunkCoordinates, instance);
+
+            return;
+        }
+
+        Set<ChunkCoordinate> finalGroup = new HashSet<>();
+
+        finalGroup.add(new ChunkCoordinate(chunkX, chunkZ));
+
+        for (Set<ChunkCoordinate> chunkCoordinates : neighboursGroups) {
+            finalGroup.addAll(chunkCoordinates);
+        }
+
+        // Complete maps
+        for (ChunkCoordinate chunkCoordinate : finalGroup) {
+            chunksGroupMap.put(chunkCoordinate, finalGroup);
+        }
+
+        instanceMap.put(finalGroup, instance);
+
     }
 
     @Override
     public void onChunkUnload(Instance instance, int chunkX, int chunkZ) {
+        Map<ChunkCoordinate, Set<ChunkCoordinate>> chunksGroupMap = getChunksGroupMap(instance);
+        Map<Set<ChunkCoordinate>, Instance> instanceMap = getInstanceMap(instance);
 
+        final ChunkCoordinate chunkCoordinate = new ChunkCoordinate(chunkX, chunkZ);
+        if (chunksGroupMap.containsKey(chunkCoordinate)) {
+            Set<ChunkCoordinate> chunkCoordinates = chunksGroupMap.get(chunkCoordinate);
+            chunkCoordinates.remove(chunkCoordinate);
+            chunksGroupMap.remove(chunkCoordinate);
+
+            if (chunkCoordinates.isEmpty()) {
+                instanceMap.entrySet().removeIf(entry -> entry.getKey().isEmpty());
+            }
+        }
     }
 
     @Override
     public void update(long time) {
-
         // Set of already-updated instances
         final Set<Instance> updatedInstance = new HashSet<>();
 
-        // Update all the chunks
-        for (Map.Entry<Set<ChunkCoordinate>, Instance> entry : instanceMap.entrySet()) {
-            Set<ChunkCoordinate> chunks = entry.getKey();
-            Instance instance = entry.getValue();
 
-            final boolean updateInstance = updatedInstance.add(instance);
+        instanceInstanceMap.entrySet().forEach(entry -> {
-            pool.execute(() -> {
+            final Instance instance = entry.getKey();
-                if (updateInstance) {
+            final Map<Set<ChunkCoordinate>, Instance> instanceMap = entry.getValue();
-                    updateInstance(instance, time);
-                }
 
-                for (ChunkCoordinate chunkCoordinate : chunks) {
+            // Update all the chunks
-                    final Chunk chunk = instance.getChunk(chunkCoordinate.chunkX, chunkCoordinate.chunkZ);
+            for (Map.Entry<Set<ChunkCoordinate>, Instance> ent : instanceMap.entrySet()) {
-                    if (ChunkUtils.isChunkUnloaded(chunk)) {
+                final Set<ChunkCoordinate> chunks = ent.getKey();
-                        continue;
+
+                final boolean updateInstance = updatedInstance.add(instance);
+                pool.execute(() -> {
+                    if (updateInstance) {
+                        updateInstance(instance, time);
                     }
 
-                    updateChunk(instance, chunk, time);
+                    for (ChunkCoordinate chunkCoordinate : chunks) {
+                        final Chunk chunk = instance.getChunk(chunkCoordinate.chunkX, chunkCoordinate.chunkZ);
+                        if (ChunkUtils.isChunkUnloaded(chunk)) {
+                            continue;
+                        }
 
-                    updateEntities(instance, chunk, time);
+                        updateChunk(instance, chunk, time);
-                }
-            });
 
-        }
+                        updateEntities(instance, chunk, time);
+                    }
+                });
+
+            }
+
+        });
     }
 
-    /*@Override
+    private List<ChunkCoordinate> getNeighbours(Instance instance, int chunkX, int chunkZ) {
-    public void linkThread(Instance instance, Chunk chunk) {
+        List<ChunkCoordinate> chunks = new ArrayList<>();
-        startChunkQuery(instance, chunk.getChunkX(), chunk.getChunkZ());
-    }*/
-
-    /**
-     * Check the four chunk neighbors (up/down/left/right)
-     * and add them to the cache list
-     *
-     * @param instance the instance which is checked
-     * @param chunkX   the chunk X
-     * @param chunkZ   the chunk Z
-     */
-    /*private void startChunkQuery(Instance instance, int chunkX, int chunkZ) {
         // Constants used to loop through the neighbors
         final int[] posX = {1, 0, -1};
         final int[] posZ = {1, 0, -1};
 
-        // The cache which will contain all the current chunk group
-        final Set<Chunk> cache = new HashSet<>();
-
         for (int x : posX) {
             for (int z : posZ) {
 
@@ -99,31 +142,24 @@ public class PerGroupChunkProvider extends ThreadProvider {
 
                 final int targetX = chunkX + x;
                 final int targetZ = chunkZ + z;
-
                 final Chunk chunk = instance.getChunk(targetX, targetZ);
-                if (cache.contains(chunk)) {
+                if (!ChunkUtils.isChunkUnloaded(chunk)) {
-                    continue;
+                    chunks.add(toChunkCoordinate(chunk));
+                } else {
+                    //System.out.println(targetX+" : "+targetZ);
                 }
 
-                if (chunk != null) {
-                    // If loaded, check if the chunk is already associated with a Set
-                    if (cachedChunks.containsKey(chunk)) {
-                        // Chunk is associated with a Set, add all them to the updated cache Set
-                        Set<Chunk> oldCache = cachedChunks.get(chunk);
-                        cache.addAll(oldCache);
-                        this.instanceMap.remove(oldCache);
-                    } else {
-                        // Chunk is alone, add it to the cache list
-                        cache.add(chunk);
-                    }
-                }
             }
         }
+        return chunks;
+    }
+
+    private Map<ChunkCoordinate, Set<ChunkCoordinate>> getChunksGroupMap(Instance instance) {
+        return instanceChunksGroupMap.computeIfAbsent(instance, inst -> new HashMap<>());
+    }
+
+    private Map<Set<ChunkCoordinate>, Instance> getInstanceMap(Instance instance) {
+        return instanceInstanceMap.computeIfAbsent(instance, inst -> new HashMap<>());
+    }
 
-        // Add cached chunks into a cache list
-        for (Chunk cachedChunk : cache) {
-            this.cachedChunks.put(cachedChunk, cache);
-        }
-        this.instanceMap.put(cache, instance);
-    }*/
 }

src/main/java/net/minestom/server/thread/PerInstanceThreadProvider.java

@@ -18,13 +18,13 @@ public class PerInstanceThreadProvider extends ThreadProvider {
 
     @Override
     public void onChunkLoad(Instance instance, int chunkX, int chunkZ) {
-        Set<ChunkCoordinate> chunkCoordinates = instanceChunkMap.computeIfAbsent(instance, inst -> new HashSet<>());
+        Set<ChunkCoordinate> chunkCoordinates = getChunkCoordinates(instance);
         chunkCoordinates.add(new ChunkCoordinate(chunkX, chunkZ));
     }
 
     @Override
     public void onChunkUnload(Instance instance, int chunkX, int chunkZ) {
-        Set<ChunkCoordinate> chunkCoordinates = instanceChunkMap.computeIfAbsent(instance, inst -> new HashSet<>());
+        Set<ChunkCoordinate> chunkCoordinates = getChunkCoordinates(instance);
 
         chunkCoordinates.removeIf(chunkCoordinate -> chunkCoordinate.chunkX == chunkX &&
                 chunkCoordinate.chunkZ == chunkZ);
@@ -55,4 +55,8 @@ public class PerInstanceThreadProvider extends ThreadProvider {
         }
     }
 
+    private Set<ChunkCoordinate> getChunkCoordinates(Instance instance) {
+        return instanceChunkMap.computeIfAbsent(instance, inst -> new HashSet<>());
+    }
+
 }

src/main/java/net/minestom/server/thread/ThreadProvider.java

@@ -6,6 +6,7 @@ import net.minestom.server.instance.Chunk;
 import net.minestom.server.instance.Instance;
 import net.minestom.server.utils.thread.MinestomThread;
 
+import java.util.Objects;
 import java.util.Set;
 import java.util.concurrent.ExecutorService;
 import java.util.function.Function;
@@ -182,6 +183,10 @@ public abstract class ThreadProvider {
         }
     }
 
+    protected ChunkCoordinate toChunkCoordinate(Chunk chunk) {
+        return new ChunkCoordinate(chunk.getChunkX(), chunk.getChunkZ());
+    }
+
     protected static class ChunkCoordinate {
         public int chunkX, chunkZ;
 
@@ -189,6 +194,20 @@ public abstract class ThreadProvider {
             this.chunkX = chunkX;
             this.chunkZ = chunkZ;
         }
+
+        @Override
+        public boolean equals(Object o) {
+            if (this == o) return true;
+            if (o == null || getClass() != o.getClass()) return false;
+            ChunkCoordinate that = (ChunkCoordinate) o;
+            return chunkX == that.chunkX &&
+                    chunkZ == that.chunkZ;
+        }
+
+        @Override
+        public int hashCode() {
+            return Objects.hash(chunkX, chunkZ);
+        }
     }
 
 }