Dirty tracking (#547)

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

@@ -132,16 +132,28 @@ public class Entity implements Viewable, Tickable, Schedulable, TagHandler, Perm
     protected final ViewEngine viewEngine = new ViewEngine(this,
             player -> {
                 // Add viewable
-                if (!Entity.this.viewEngine.viewableOption.predicate(player) ||
+                var lock1 = player.getEntityId() < getEntityId() ? player : this;
-                        !player.viewEngine.viewerOption.predicate(this)) return;
+                var lock2 = lock1 == this ? player : this;
-                Entity.this.viewEngine.viewableOption.register(player);
+                synchronized (lock1.viewEngine.mutex()) {
-                player.viewEngine.viewerOption.register(this);
+                    synchronized (lock2.viewEngine.mutex()) {
+                        if (!Entity.this.viewEngine.viewableOption.predicate(player) ||
+                                !player.viewEngine.viewerOption.predicate(this)) return;
+                        Entity.this.viewEngine.viewableOption.register(player);
+                        player.viewEngine.viewerOption.register(this);
+                    }
+                }
                 updateNewViewer(player);
             },
             player -> {
                 // Remove viewable
-                Entity.this.viewEngine.viewableOption.unregister(player);
+                var lock1 = player.getEntityId() < getEntityId() ? player : this;
-                player.viewEngine.viewerOption.unregister(this);
+                var lock2 = lock1 == this ? player : this;
+                synchronized (lock1.viewEngine.mutex()) {
+                    synchronized (lock2.viewEngine.mutex()) {
+                        Entity.this.viewEngine.viewableOption.unregister(player);
+                        player.viewEngine.viewerOption.unregister(this);
+                    }
+                }
                 updateOldViewer(player);
             },
             this instanceof Player player ? entity -> entity.viewEngine.viewableOption.addition.accept(player) : null,

src/main/java/net/minestom/server/instance/EntityTracker.java

@@ -5,7 +5,10 @@ import net.minestom.server.entity.Entity;
 import net.minestom.server.entity.ExperienceOrb;
 import net.minestom.server.entity.ItemEntity;
 import net.minestom.server.entity.Player;
-import org.jetbrains.annotations.*;
+import org.jetbrains.annotations.ApiStatus;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+import org.jetbrains.annotations.UnmodifiableView;
 
 import java.util.Collection;
 import java.util.List;
@@ -52,25 +55,14 @@ public sealed interface EntityTracker permits EntityTrackerImpl {
         difference(from.chunkX(), from.chunkZ(), to.chunkX(), to.chunkZ(), target, update);
     }
 
-    /**
-     * Gets the entities present in the specified chunk.
-     */
-    <T extends Entity> void chunkEntities(int chunkX, int chunkZ,
-                                          @NotNull Target<T> target, @NotNull Query<T> query);
-
-    default <T extends Entity> void chunkEntities(@NotNull Point point,
-                                                  @NotNull Target<T> target, @NotNull Query<T> query) {
-        chunkEntities(point.chunkX(), point.chunkZ(), target, query);
-    }
-
     /**
      * Returns a copy of the entities present in the specified chunk.
      */
     @ApiStatus.Experimental
-    @Unmodifiable <T extends Entity> Collection<T> chunkEntities(int chunkX, int chunkZ, @NotNull Target<T> target);
+    @UnmodifiableView <T extends Entity> Collection<T> chunkEntities(int chunkX, int chunkZ, @NotNull Target<T> target);
 
     @ApiStatus.Experimental
-    @Unmodifiable
+    @UnmodifiableView
     default <T extends Entity> @NotNull Collection<T> chunkEntities(@NotNull Point point, @NotNull Target<T> target) {
         return chunkEntities(point.chunkX(), point.chunkZ(), target);
     }
@@ -114,11 +106,6 @@ public sealed interface EntityTracker permits EntityTrackerImpl {
         return entities(Target.ENTITIES);
     }
 
-    /**
-     * Run {@code runnable} and ensure that the tracking state is locked during execution.
-     */
-    void synchronize(@NotNull Point point, @NotNull Runnable runnable);
-
     /**
      * Represents the type of entity you want to retrieve.
      *

src/main/java/net/minestom/server/instance/EntityTrackerImpl.java

@@ -1,9 +1,5 @@
 package net.minestom.server.instance;
 
-import it.unimi.dsi.fastutil.ints.Int2ObjectMap;
-import it.unimi.dsi.fastutil.ints.Int2ObjectOpenHashMap;
-import it.unimi.dsi.fastutil.longs.Long2ObjectMap;
-import it.unimi.dsi.fastutil.longs.Long2ObjectOpenHashMap;
 import net.minestom.server.MinecraftServer;
 import net.minestom.server.coordinate.Point;
 import net.minestom.server.entity.Entity;
@@ -12,6 +8,8 @@ import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.jetbrains.annotations.Unmodifiable;
 import org.jetbrains.annotations.UnmodifiableView;
+import space.vectrix.flare.fastutil.Int2ObjectSyncMap;
+import space.vectrix.flare.fastutil.Long2ObjectSyncMap;
 
 import java.util.*;
 import java.util.concurrent.ConcurrentHashMap;
@@ -31,11 +29,11 @@ final class EntityTrackerImpl implements EntityTracker {
     // The array index is the Target enum ordinal
     private final TargetEntry<Entity>[] entries = EntityTracker.Target.TARGETS.stream().map((Function<Target<?>, TargetEntry>) TargetEntry::new).toArray(TargetEntry[]::new);
 
-    private final Int2ObjectMap<Point> entityPositions = new Int2ObjectOpenHashMap<>();
+    private final Int2ObjectSyncMap<Point> entityPositions = Int2ObjectSyncMap.hashmap();
 
     @Override
-    public synchronized <T extends Entity> void register(@NotNull Entity entity, @NotNull Point point,
+    public <T extends Entity> void register(@NotNull Entity entity, @NotNull Point point,
-                                                         @NotNull Target<T> target, @Nullable Update<T> update) {
+                                            @NotNull Target<T> target, @Nullable Update<T> update) {
         var prevPoint = entityPositions.putIfAbsent(entity.getEntityId(), point);
         if (prevPoint != null) return;
         final long index = getChunkIndex(point);
@@ -54,8 +52,8 @@ final class EntityTrackerImpl implements EntityTracker {
     }
 
     @Override
-    public synchronized <T extends Entity> void unregister(@NotNull Entity entity,
+    public <T extends Entity> void unregister(@NotNull Entity entity,
-                                                           @NotNull Target<T> target, @Nullable Update<T> update) {
+                                              @NotNull Target<T> target, @Nullable Update<T> update) {
         final Point point = entityPositions.remove(entity.getEntityId());
         if (point == null) return;
         final long index = getChunkIndex(point);
@@ -71,8 +69,8 @@ final class EntityTrackerImpl implements EntityTracker {
     }
 
     @Override
-    public synchronized <T extends Entity> void move(@NotNull Entity entity, @NotNull Point newPoint,
+    public <T extends Entity> void move(@NotNull Entity entity, @NotNull Point newPoint,
-                                                     @NotNull Target<T> target, @Nullable Update<T> update) {
+                                        @NotNull Target<T> target, @Nullable Update<T> update) {
         Point oldPoint = entityPositions.put(entity.getEntityId(), newPoint);
         if (oldPoint == null || oldPoint.sameChunk(newPoint)) return;
         final long oldIndex = getChunkIndex(oldPoint);
@@ -99,9 +97,9 @@ final class EntityTrackerImpl implements EntityTracker {
     }
 
     @Override
-    public synchronized <T extends Entity> void difference(int oldChunkX, int oldChunkZ,
+    public <T extends Entity> void difference(int oldChunkX, int oldChunkZ,
-                                                           int newChunkX, int newChunkZ,
+                                              int newChunkX, int newChunkZ,
-                                                           @NotNull Target<T> target, @NotNull Update<T> update) {
+                                              @NotNull Target<T> target, @NotNull Update<T> update) {
         final TargetEntry<Entity> entry = entries[target.ordinal()];
         forDifferingChunksInRange(newChunkX, newChunkZ, oldChunkX, oldChunkZ,
                 MinecraftServer.getEntityViewDistance(), (chunkX, chunkZ) -> {
@@ -118,18 +116,10 @@ final class EntityTrackerImpl implements EntityTracker {
     }
 
     @Override
-    public synchronized <T extends Entity> void chunkEntities(int chunkX, int chunkZ, @NotNull Target<T> target, @NotNull Query<T> query) {
+    public @Unmodifiable <T extends Entity> Collection<T> chunkEntities(int chunkX, int chunkZ, @NotNull Target<T> target) {
         final TargetEntry<Entity> entry = entries[target.ordinal()];
-        final List<Entity> entities = entry.chunkEntities.get(getChunkIndex(chunkX, chunkZ));
+        //noinspection unchecked
-        if (entities == null || entities.isEmpty()) return;
+        return (Collection<T>) entry.chunkEntities.computeIfAbsent(getChunkIndex(chunkX, chunkZ), i -> LIST_SUPPLIER.get());
-        for (Entity entity : entities) query.consume((T) entity);
-    }
-
-    @Override
-    public synchronized @Unmodifiable <T extends Entity> Collection<T> chunkEntities(int chunkX, int chunkZ, @NotNull Target<T> target) {
-        final TargetEntry<Entity> entry = entries[target.ordinal()];
-        final List<Entity> entities = entry.chunkEntities.get(getChunkIndex(chunkX, chunkZ));
-        return entities != null ? (Collection<T>) List.copyOf(entities) : List.of();
     }
 
     @Override
@@ -141,7 +131,7 @@ final class EntityTrackerImpl implements EntityTracker {
     }
 
     @Override
-    public synchronized @NotNull <T extends Entity> List<List<T>> references(int chunkX, int chunkZ, @NotNull Target<T> target) {
+    public @NotNull <T extends Entity> List<List<T>> references(int chunkX, int chunkZ, @NotNull Target<T> target) {
         // Gets reference to all chunk entities lists within the range
         // This is used to avoid a map lookup per chunk
         final TargetEntry<T> entry = (TargetEntry<T>) entries[target.ordinal()];
@@ -156,15 +146,17 @@ final class EntityTrackerImpl implements EntityTracker {
     }
 
     @Override
-    public synchronized <T extends Entity> void nearbyEntities(@NotNull Point point, double range, @NotNull Target<T> target, @NotNull Query<T> query) {
+    public <T extends Entity> void nearbyEntities(@NotNull Point point, double range, @NotNull Target<T> target, @NotNull Query<T> query) {
         final int chunkRange = Math.abs((int) (range / Chunk.CHUNK_SECTION_SIZE)) + 1;
         final double squaredRange = range * range;
-        ChunkUtils.forChunksInRange(point, chunkRange, (chunkX, chunkZ) ->
+        ChunkUtils.forChunksInRange(point, chunkRange, (chunkX, chunkZ) -> {
-                chunkEntities(chunkX, chunkZ, target, entity -> {
+            var chunkEntities = chunkEntities(chunkX, chunkZ, target);
-                    if (point.distanceSquared(entity.getPosition()) < squaredRange) {
+            chunkEntities.forEach(entity -> {
-                        query.consume(entity);
+                if (point.distanceSquared(entity.getPosition()) < squaredRange) {
-                    }
+                    query.consume(entity);
-                }));
+                }
+            });
+        });
     }
 
     @Override
@@ -173,19 +165,14 @@ final class EntityTrackerImpl implements EntityTracker {
         return (Set<T>) entries[target.ordinal()].entitiesView;
     }
 
-    @Override
-    public synchronized void synchronize(@NotNull Point point, @NotNull Runnable runnable) {
-        runnable.run();
-    }
-
     private static final class TargetEntry<T extends Entity> {
         private final EntityTracker.Target<T> target;
         private final Set<T> entities = ConcurrentHashMap.newKeySet(); // Thread-safe since exposed
         private final Set<T> entitiesView = Collections.unmodifiableSet(entities);
         // Chunk index -> entities inside it
-        private final Long2ObjectMap<List<T>> chunkEntities = new Long2ObjectOpenHashMap<>(0);
+        private final Long2ObjectSyncMap<List<T>> chunkEntities = Long2ObjectSyncMap.hashmap();
         // Chunk index -> lists of visible entities (references to chunkEntities entries)
-        private final Long2ObjectMap<List<List<T>>> chunkRangeEntities = new Long2ObjectOpenHashMap<>(0);
+        private final Long2ObjectSyncMap<List<List<T>>> chunkRangeEntities = Long2ObjectSyncMap.hashmap();
 
         TargetEntry(Target<T> target) {
             this.target = target;

src/main/java/net/minestom/server/instance/Instance.java

@@ -1,5 +1,6 @@
 package net.minestom.server.instance;
 
+import it.unimi.dsi.fastutil.objects.ObjectArraySet;
 import net.kyori.adventure.identity.Identity;
 import net.kyori.adventure.pointer.Pointers;
 import net.minestom.server.MinecraftServer;
@@ -486,9 +487,8 @@ public abstract class Instance implements Block.Getter, Block.Setter, Tickable,
      * if {@code chunk} is unloaded, return an empty {@link HashSet}
      */
     public @NotNull Set<@NotNull Entity> getChunkEntities(Chunk chunk) {
-        Set<Entity> result = new HashSet<>();
+        var chunkEntities = entityTracker.chunkEntities(chunk.toPosition(), EntityTracker.Target.ENTITIES);
-        this.entityTracker.chunkEntities(chunk.toPosition(), EntityTracker.Target.ENTITIES, result::add);
+        return ObjectArraySet.ofUnchecked(chunkEntities.toArray(Entity[]::new));
-        return result;
     }
 
     /**

src/main/java/net/minestom/server/instance/InstanceContainer.java

@@ -219,7 +219,7 @@ public class InstanceContainer extends Instance {
 
         EventDispatcher.call(new InstanceChunkUnloadEvent(this, chunk));
         // Remove all entities in chunk
-        getEntityTracker().chunkEntities(chunkX, chunkZ, EntityTracker.Target.ENTITIES, Entity::remove);
+        getEntityTracker().chunkEntities(chunkX, chunkZ, EntityTracker.Target.ENTITIES).forEach(Entity::remove);
         // Clear cache
         this.chunks.remove(index);
         chunk.unload();

src/main/java/net/minestom/server/utils/ViewEngine.java

@@ -2,7 +2,6 @@ package net.minestom.server.utils;
 
 import it.unimi.dsi.fastutil.ints.IntOpenHashSet;
 import it.unimi.dsi.fastutil.ints.IntSet;
-import it.unimi.dsi.fastutil.objects.ObjectArraySet;
 import net.minestom.server.coordinate.Point;
 import net.minestom.server.entity.Entity;
 import net.minestom.server.entity.Player;
@@ -12,6 +11,7 @@ import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
 import java.util.*;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicIntegerFieldUpdater;
 import java.util.function.Consumer;
@@ -23,7 +23,7 @@ import java.util.function.Predicate;
 @ApiStatus.Internal
 public final class ViewEngine {
     private final Entity entity;
-    private final ObjectArraySet<Player> manualViewers = new ObjectArraySet<>();
+    private final Set<Player> manualViewers = ConcurrentHashMap.newKeySet();
 
     private EntityTracker tracker;
     private Point lastTrackingPoint;
@@ -112,6 +112,10 @@ public final class ViewEngine {
         return entity == null || viewableOption.isRegistered(player);
     }
 
+    public Object mutex() {
+        return mutex;
+    }
+
     public Set<Player> asSet() {
         return set;
     }
@@ -192,17 +196,15 @@ public final class ViewEngine {
                             Predicate<T> visibilityPredicate,
                             Consumer<T> action) {
             if (tracker == null || references == null) return;
-            tracker.synchronize(lastTrackingPoint, () -> {
+            for (List<T> entities : references) {
-                for (List<T> entities : references) {
+                if (entities.isEmpty()) continue;
-                    if (entities.isEmpty()) continue;
+                for (T entity : entities) {
-                    for (T entity : entities) {
+                    if (entity == ViewEngine.this.entity || !visibilityPredicate.test(entity)) continue;
-                        if (entity == ViewEngine.this.entity || !visibilityPredicate.test(entity)) continue;
+                    if (entity instanceof Player player && manualViewers.contains(player)) continue;
-                        if (entity instanceof Player player && manualViewers.contains(player)) continue;
+                    if (entity.getVehicle() != null) continue;
-                        if (entity.getVehicle() != null) continue;
+                    action.accept(entity);
-                        action.accept(entity);
-                    }
                 }
-            });
+            }
         }
     }