Fix 5 chunk-pipeline performance hotspots

engine/src/main/java/org/terasology/engine/core/GameThread.java

@@ -42,7 +42,9 @@ public static boolean isCurrentThread() {
      */
     public static void asynch(Runnable process) {
         if (!Thread.currentThread().equals(gameThread)) {
-            pendingRunnables.push(process);
+            // addLast (not push/addFirst): processWaitingProcesses() drains head-first,
+            // so submissions must queue at the tail to run in FIFO order.
+            pendingRunnables.addLast(process);
         } else {
             process.run();
         }
@@ -58,7 +60,7 @@ public static void asynch(Runnable process) {
     public static void synch(Runnable process) throws InterruptedException {
         if (!Thread.currentThread().equals(gameThread)) {
             BlockingProcess blockingProcess = new BlockingProcess(process);
-            pendingRunnables.push(blockingProcess);
+            pendingRunnables.addLast(blockingProcess);
             blockingProcess.waitForCompletion();
         } else {
             process.run();

engine/src/main/java/org/terasology/engine/rendering/primitives/ChunkTessellator.java

@@ -31,6 +31,21 @@ public ChunkMesh generateMesh(ChunkView chunkView, float scale, int border) {
         PerformanceMonitor.startActivity("GenerateMesh");
         ChunkMeshImpl mesh = new ChunkMeshImpl();
 
+        // Each render type's vertex/index buffers start at zero capacity and grow by doubling, every
+        // growth step being a fresh native ByteBuffer allocation plus a copy of everything written so
+        // far. Seeding a modest initial reservation - one XZ layer's worth of quads, a real property
+        // of the chunk rather than a tuned number - skips the cheapest-but-most-frequent early growth
+        // steps for typical terrain, without meaningfully over-reserving near-empty (all-air or
+        // all-solid-interior) chunks.
+        // Pooling/reusing these buffers instead of allocating fresh each call would help further, but
+        // needs real lifecycle coordination with the GL upload (a separate thread) - not done here.
+        int initialVertexReservation = Chunks.SIZE_X * Chunks.SIZE_Z;
+        for (ChunkMesh.RenderType type : ChunkMesh.RenderType.values()) {
+            ChunkMesh.VertexElements elements = mesh.getVertexElements(type);
+            elements.buffer.reserveElements(initialVertexReservation);
+            elements.indices.reserveElements(initialVertexReservation);
+        }
+
         final Stopwatch watch = Stopwatch.createStarted();
 
         // The mesh extends into the borders in the horizontal directions, but not vertically upwards, in order to cover

engine/src/main/java/org/terasology/engine/rendering/world/RenderableWorldImpl.java

@@ -411,17 +411,16 @@
     }
 
     private static float squaredDistanceToCamera(RenderableChunk chunk, Vector3f cameraPosition) {
-        // For performance reasons, to avoid instantiating too many vectors in a frequently called method,
-        // comments are in use instead of appropriately named vectors.
-        Vector3f result = chunk.getRenderPosition();
-        result.add(CHUNK_CENTER_OFFSET);
-
-        result.sub(cameraPosition); // camera to chunk vector
-
-        return result.lengthSquared();
+        // distanceSquared() lets non-default implementations (Chunk, LodChunk) compute this from
+        // their raw offset fields, avoiding a getRenderPosition() Vector3f allocation per call -
+        // this runs twice per PriorityQueue comparison, every frame.
+        return chunk.distanceSquared(
+                cameraPosition.x - CHUNK_CENTER_OFFSET.x(),
+                cameraPosition.y - CHUNK_CENTER_OFFSET.y(),
+                cameraPosition.z - CHUNK_CENTER_OFFSET.z());
     }
 
     // TODO: find the right place to check if the activeCamera has changed,
     // TODO: so that the comparators can hold an up-to-date reference to it
     // TODO: and avoid having to find it on a per-comparison basis.
     public static class ChunkFrontToBackComparator implements Comparator<RenderableChunk> {

engine/src/main/java/org/terasology/engine/world/block/shapes/BlockMeshPart.java

@@ -83,14 +83,15 @@ public BlockMeshPart mapTexCoords(Vector2f offset, float width, int frames) {
     public void appendTo(ChunkMesh chunk, ChunkView chunkView, int offsetX, int offsetY, int offsetZ,
                          ChunkMesh.RenderType renderType, Colorc colorOffset, ChunkVertexFlag flags) {
         ChunkMesh.VertexElements elements = chunk.getVertexElements(renderType);
-        for (Vector2f texCoord : texCoords) {
-            elements.uv0.put(texCoord);
-        }
 
         int nextIndex = elements.vertexCount;
         elements.buffer.reserveElements(nextIndex + vertices.length);
         Vector3f pos = new Vector3f();
+        // uv0 used to be written in its own pass before this loop; texCoords[vIdx] lines up with
+        // vertices[vIdx] the same as every other per-vertex attribute here, so it belongs in this
+        // loop too - this runs per visible block face, so the extra full pass wasn't free.
         for (int vIdx = 0; vIdx < vertices.length; ++vIdx) {
+            elements.uv0.put(texCoords[vIdx]);
             elements.color.put(colorOffset);
             elements.position.put(pos.set(vertices[vIdx]).add(offsetX, offsetY, offsetZ));
             elements.normals.put(normals[vIdx]);

engine/src/main/java/org/terasology/engine/world/chunks/Chunk.java

@@ -141,6 +141,14 @@ default Vector3f getRenderPosition() {
         return new Vector3f(getChunkWorldOffsetX(), getChunkWorldOffsetY(), getChunkWorldOffsetZ());
     }
 
+    @Override
+    default float distanceSquared(float x, float y, float z) {
+        float dx = getChunkWorldOffsetX() - x;
+        float dy = getChunkWorldOffsetY() - y;
+        float dz = getChunkWorldOffsetZ() - z;
+        return dx * dx + dy * dy + dz * dz;
+    }
+
 
     /**
      * Returns X offset of this chunk to the world center (0:0:0), with one unit being one block.

engine/src/main/java/org/terasology/engine/world/chunks/LodChunk.java

@@ -38,6 +38,14 @@ public Vector3f getRenderPosition() {
         return new Vector3f(position).mul(Chunks.SIZE_X, Chunks.SIZE_Y, Chunks.SIZE_Z);
     }
 
+    @Override
+    public float distanceSquared(float x, float y, float z) {
+        float dx = position.x() * Chunks.SIZE_X - x;
+        float dy = position.y() * Chunks.SIZE_Y - y;
+        float dz = position.z() * Chunks.SIZE_Z - z;
+        return dx * dx + dy * dy + dz * dz;
+    }
+
     @Override
     public AABBfc getAABB() {
         Vector3f min = getRenderPosition();

engine/src/main/java/org/terasology/engine/world/chunks/RenderableChunk.java

@@ -15,6 +15,19 @@ public interface RenderableChunk {
 
     Vector3f getRenderPosition();
 
+    /**
+     * Squared distance from this chunk's render position to the given point, without allocating
+     * a temporary vector. Implementations that can compute their render position from primitive
+     * fields should override this; the default falls back to {@link #getRenderPosition()}.
+     */
+    default float distanceSquared(float x, float y, float z) {
+        Vector3f renderPosition = getRenderPosition();
+        float dx = renderPosition.x - x;
+        float dy = renderPosition.y - y;
+        float dz = renderPosition.z - z;
+        return dx * dx + dy * dy + dz * dz;
+    }
+
     AABBfc getAABB();
 
     void setMesh(ChunkMesh newMesh);

engine/src/main/java/org/terasology/engine/world/chunks/localChunkProvider/RelevanceSystem.java

@@ -26,13 +26,13 @@
 import org.terasology.gestalt.entitysystem.event.ReceiveEvent;
 
 import javax.inject.Inject;
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Comparator;
+import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.concurrent.Future;
-import java.util.concurrent.locks.ReadWriteLock;
-import java.util.concurrent.locks.ReentrantReadWriteLock;
-import java.util.stream.StreamSupport;
 
 /**
  * RelevanceSystem loads, holds and unloads chunks around "players" (entity with {@link RelevanceRegionComponent} and
@@ -47,8 +47,11 @@
 public class RelevanceSystem implements UpdateSubscriberSystem {
 
     private static final Vector3i UNLOAD_LEEWAY = new Vector3i(1, 1, 1);
-    private final ReadWriteLock regionLock = new ReentrantReadWriteLock();
-    private final Map<EntityRef, ChunkRelevanceRegion> regions = Maps.newHashMap();
+    // ConcurrentHashMap rather than a HashMap guarded by a lock: regionsDistanceScore() is called
+    // from ChunkTaskRelevanceComparator, on every PriorityBlockingQueue comparison in the chunk
+    // processing pipeline - a lock acquired there for every comparison is pure overhead compared to
+    // a lock-free read.
+    private final Map<EntityRef, ChunkRelevanceRegion> regions = Maps.newConcurrentMap();
     private final LocalChunkProvider chunkProvider;
 
     @Inject
@@ -96,14 +99,9 @@ public void onRemoveChunk(BeforeChunkUnload chunkUnloadEvent, EntityRef worldEnt
      * @param distance new distance for setting to entity's region.
      */
     public void updateRelevanceEntityDistance(EntityRef entity, Vector3ic distance) {
-        regionLock.readLock().lock();
-        try {
-            ChunkRelevanceRegion region = regions.get(entity);
-            if (region != null) {
-                region.setRelevanceDistance(distance);
-            }
-        } finally {
-            regionLock.readLock().unlock();
+        ChunkRelevanceRegion region = regions.get(entity);
+        if (region != null) {
+            region.setRelevanceDistance(distance);
         }
     }
 
@@ -113,12 +111,7 @@ public void updateRelevanceEntityDistance(EntityRef entity, Vector3ic distance)
      * @param entity entity for remove.
      */
     public void removeRelevanceEntity(EntityRef entity) {
-        regionLock.writeLock().lock();
-        try {
-            regions.remove(entity);
-        } finally {
-            regionLock.writeLock().unlock();
-        }
+        regions.remove(entity);
     }
 
     /**
@@ -157,39 +150,37 @@ public BlockRegionc addRelevanceEntity(EntityRef entity, Vector3ic distance, Chu
         if (!entity.exists()) {
             return null;  // Futures.immediateFailedFuture(new IllegalArgumentException("Entity does not exist."));
         }
-        regionLock.readLock().lock();
-        try {
-            ChunkRelevanceRegion region = regions.get(entity);
-            if (region != null) {
-                region.setRelevanceDistance(distance);
-                return new BlockRegion(region.getCurrentRegion());  // Future of “when region.currentRegion is no longer dirty”?
-            }
-        } finally {
-            regionLock.readLock().unlock();
+        ChunkRelevanceRegion existing = regions.get(entity);
+        if (existing != null) {
+            existing.setRelevanceDistance(distance);
+            return new BlockRegion(existing.getCurrentRegion());  // Future of “when region.currentRegion is no longer dirty”?
         }
         ChunkRelevanceRegion region = new ChunkRelevanceRegion(entity, distance);
         if (listener != null) {
             region.setListener(listener);
         }
-        regionLock.writeLock().lock();
-        try {
-            regions.put(entity, region);
-        } finally {
-            regionLock.writeLock().unlock();
+        regions.put(entity, region);
+
+        // BlockRegion's iterator hands back the same mutable Vector3i every call (mutated in place) -
+        // safe for immediate per-element use, but not for buffering, which .sorted() must do. Each
+        // position is copied here before it's reused for the next one, and its score is computed once
+        // up front rather than repeatedly during the sort's comparisons.
+        List<Vector3ic> positions = new ArrayList<>();
+        region.getCurrentRegion().forEach(pos -> positions.add(new Vector3i(pos)));
+        Map<Vector3ic, Integer> relevanceScores = new HashMap<>(positions.size());
+        for (Vector3ic pos : positions) {
+            relevanceScores.put(pos, regionsDistanceScore(pos));
+        }
+        positions.sort(Comparator.comparingInt(relevanceScores::get));
+        for (Vector3ic pos : positions) {
+            Chunk chunk = chunkProvider.getChunk(pos);
+            if (chunk != null) {
+                region.checkIfChunkIsRelevant(chunk);
+                // return Futures.immediateFuture(chunk);
+            } else {
+                chunkProvider.createOrLoadChunk(pos); // return this
+            }
         }
-
-        StreamSupport.stream(region.getCurrentRegion().spliterator(), false)
-                .sorted(new PositionRelevanceComparator()) //<-- this is n^2 cost. not sure why this needs to be sorted like this.
-                .forEach(pos -> {
-                            Chunk chunk = chunkProvider.getChunk(pos);
-                            if (chunk != null) {
-                                region.checkIfChunkIsRelevant(chunk);
-                                // return Futures.immediateFuture(chunk);
-                            } else {
-                                chunkProvider.createOrLoadChunk(pos); // return this
-                            }
-                        }
-                );
         return new BlockRegion(region.getCurrentRegion());  // whenAllComplete
     }
 
@@ -257,23 +248,16 @@ public void shutdown() {
 
     private int regionsDistanceScore(Vector3ic chunk) {
         int score = Integer.MAX_VALUE;
-
-        regionLock.readLock().lock();
-        try {
-
-            for (ChunkRelevanceRegion region : regions.values()) {
-                int dist = (int) chunk.gridDistance(region.getCenter());
-                if (dist < score) {
-                    score = dist;
-                }
-                if (score == 0) {
-                    break;
-                }
+        for (ChunkRelevanceRegion region : regions.values()) {
+            int dist = (int) chunk.gridDistance(region.getCenter());
+            if (dist < score) {
+                score = dist;
+            }
+            if (score == 0) {
+                break;
             }
-            return score;
-        } finally {
-            regionLock.readLock().unlock();
         }
+        return score;
     }
 
     /**
@@ -290,20 +274,4 @@ private int score(PositionFuture<?> task) {
             return RelevanceSystem.this.regionsDistanceScore(task.getPosition());
         }
     }
-
-
-    /**
-     * Compare ChunkTasks by distance from region's centers.
-     */
-    private class PositionRelevanceComparator implements Comparator<Vector3ic> {
-
-        @Override
-        public int compare(Vector3ic o1, Vector3ic o2) {
-            return score(o1) - score(o2);
-        }
-
-        private int score(Vector3ic position) {
-            return RelevanceSystem.this.regionsDistanceScore(position);
-        }
-    }
 }

engine/src/main/java/org/terasology/engine/world/chunks/pipeline/ChunkProcessingPipeline.java

@@ -139,12 +139,18 @@ private void onStageDone(PositionFuture<Chunk> future, ChunkProcessingInfo chunk
             if (chunkProcessingInfo.hasNextStage(stages)) {
                 chunkProcessingInfo.nextStage(stages);
                 chunkProcessingInfo.makeChunkTask();
+                // Only this one info can newly have a pending (chunkTask set, no future yet) state -
+                // resetTaskState() just cleared it, and makeChunkTask() is the only place that sets it.
+                processChunkInfo(chunkProcessingInfo);
             } else {
                 // haven't next stage
                 chunkProcessingInfo.endProcessing();
                 cleanup(chunkProcessingInfo);
             }
-            processChunkTasks();
+            // Either branch may have just satisfied a neighbour's requirement (reached a stage far
+            // enough along, or finished and become visible via chunkProvider) - retry everyone who
+            // was blocked on that, rather than rescanning every in-flight chunk.
+            retryBlockedPositions();
 
         } catch (ExecutionException e) {
             String stageName =
@@ -157,9 +163,20 @@ private void onStageDone(PositionFuture<Chunk> future, ChunkProcessingInfo chunk
         }
     }
 
-    private void processChunkTasks() {
-        for (ChunkProcessingInfo info : chunkProcessingInfoMap.values()) {
-            processChunkInfo(info);
+    /**
+     * Retry every position blocked on a missing requirement. A position only ever lands here because
+     * some other chunk hadn't reached the stage (or existence) it needed - and this fires right after
+     * a stage completion or chunk finish, which is the only thing that can have changed that.
+     */
+    private void retryBlockedPositions() {
+        if (blockedPositions.isEmpty()) {
+            return;
+        }
+        for (Vector3ic pos : Lists.newArrayList(blockedPositions)) {
+            ChunkProcessingInfo info = chunkProcessingInfoMap.get(pos);
+            if (info != null) {
+                processChunkInfo(info);
+            }
         }
     }