Optimize LightList hot path copies

8Keep · 8Keep · commit 5fe394eac9a9 · 2026-05-28T16:01:55.000-04:00
diff --git a/jme3-core/src/main/java/com/jme3/light/LightList.java b/jme3-core/src/main/java/com/jme3/light/LightList.java
@@ -49,8 +49,8 @@
 public final class LightList implements Iterable<Light>, Savable, Cloneable, JmeCloneable {
 
     private Light[] list, tlist;
-    private float[] distToOwner;
     private int listSize;
+    private int tlistSize;
     private Spatial owner;
 
     private static final int DEFAULT_SIZE = 1;
@@ -84,8 +84,6 @@ protected LightList() {
     public LightList(Spatial owner) {
         listSize = 0;
         list = new Light[DEFAULT_SIZE];
-        distToOwner = new float[DEFAULT_SIZE];
-        Arrays.fill(distToOwner, Float.NEGATIVE_INFINITY);
         this.owner = owner;
     }
 
@@ -100,11 +98,8 @@ public void setOwner(Spatial owner) {
 
     private void doubleSize() {
         Light[] temp = new Light[list.length * 2];
-        float[] temp2 = new float[list.length * 2];
         System.arraycopy(list, 0, temp, 0, list.length);
-        System.arraycopy(distToOwner, 0, temp2, 0, list.length);
         list = temp;
-        distToOwner = temp2;
     }
 
     /**
@@ -117,8 +112,7 @@ public void add(Light l) {
         if (listSize == list.length) {
             doubleSize();
         }
-        list[listSize] = l;
-        distToOwner[listSize++] = Float.NEGATIVE_INFINITY;
+        list[listSize++] = l;
     }
 
     /**
@@ -136,8 +130,9 @@ public void remove(int index) {
             return;
         }
 
-        for (int i = index; i < listSize; i++) {
-            list[i] = list[i+1];
+        int copyLength = listSize - index;
+        if (copyLength > 0) {
+            System.arraycopy(list, index + 1, list, index, copyLength);
         }
         list[listSize] = null;
     }
@@ -182,11 +177,11 @@ public void clear() {
         if (listSize == 0)
             return;
 
-        for (int i = 0; i < listSize; i++)
-            list[i] = null;
+        Arrays.fill(list, 0, listSize, null);
 
         if (tlist != null)
-            Arrays.fill(tlist, null);
+            Arrays.fill(tlist, 0, tlistSize, null);
+        tlistSize = 0;
 
         listSize = 0;
     }
@@ -205,11 +200,15 @@ public void clear() {
     public void sort(boolean transformChanged) {
         if (listSize > 1) {
             // resize or populate our temporary array as necessary
-            if (tlist == null || tlist.length != list.length) {
-                tlist = list.clone();
+            if (tlist == null || tlist.length < listSize) {
+                tlist = new Light[listSize];
             } else {
-                System.arraycopy(list, 0, tlist, 0, list.length);
+                if (tlistSize > listSize) {
+                    Arrays.fill(tlist, listSize, tlistSize, null);
+                }
             }
+            System.arraycopy(list, 0, tlist, 0, listSize);
+            tlistSize = listSize;
 
             if (transformChanged) {
                 // check distance of each light
@@ -249,31 +248,33 @@ public void update(LightList local, LightList parent, Predicate<Light> filter) {
         // using the arguments
         clear();
 
-        while (list.length <= local.listSize) {
+        int requiredSize = local.listSize + (parent == null ? 0 : parent.listSize);
+        while (list.length < requiredSize) {
             doubleSize();
         }
         
-        int localListSize = 0;
-        for(int i=0;i<local.listSize;i++){
-            Light l = local.list[i];
-            if (filter != null && !filter.test(l))  continue; 
-            list[localListSize] = l;
-            distToOwner[localListSize] = Float.NEGATIVE_INFINITY;
-            localListSize++;
+        int localListSize;
+        if (filter == null) {
+            localListSize = local.listSize;
+            System.arraycopy(local.list, 0, list, 0, localListSize);
+        } else {
+            localListSize = 0;
+            for(int i=0;i<local.listSize;i++){
+                Light l = local.list[i];
+                if (!filter.test(l))  continue;
+                list[localListSize] = l;
+                localListSize++;
+            }
         }
 
         // if the spatial has a parent node, add the lights
         // from the parent list as well
         if (parent != null) {
             int sz = localListSize + parent.listSize;
-            while (list.length <= sz)
+            while (list.length < sz)
                 doubleSize();
 
-            for (int i = 0; i < parent.listSize; i++) {
-                int p = i + localListSize;
-                list[p] = parent.list[i];
-                distToOwner[p] = Float.NEGATIVE_INFINITY;
-            }
+            System.arraycopy(parent.list, 0, list, localListSize, parent.listSize);
 
             listSize = localListSize + parent.listSize;
         } else {
@@ -319,7 +320,6 @@ public LightList clone() {
 
             clone.owner = null;
             clone.list = list.clone();
-            clone.distToOwner = distToOwner.clone();
             clone.tlist = null; // list used for sorting only
 
             return clone;
@@ -343,7 +343,6 @@ public LightList jmeClone() {
     public void cloneFields(Cloner cloner, Object original) {
         this.owner = cloner.clone(owner);
         this.list = cloner.clone(list);
-        this.distToOwner = cloner.clone(distToOwner);
     }
 
     @Override
@@ -370,13 +369,10 @@ public void read(JmeImporter im) throws IOException {
         // NOTE: make sure the array has a length of at least 1
         int arraySize = Math.max(DEFAULT_SIZE, listSize);
         list = new Light[arraySize];
-        distToOwner = new float[arraySize];
 
         for (int i = 0; i < listSize; i++) {
             list[i] = lights.get(i);
         }
-
-        Arrays.fill(distToOwner, Float.NEGATIVE_INFINITY);
     }
 
     @Override
diff --git a/jme3-core/src/test/java/com/jme3/light/LightListTest.java b/jme3-core/src/test/java/com/jme3/light/LightListTest.java
@@ -0,0 +1,156 @@
+/*
+ * Copyright (c) 2026 jMonkeyEngine
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * * Redistributions in binary form must reproduce the above copyright
+ *   notice, this list of conditions and the following disclaimer in the
+ *   documentation and/or other materials provided with the distribution.
+ *
+ * * Neither the name of 'jMonkeyEngine' nor the names of its contributors
+ *   may be used to endorse or promote products derived from this software
+ *   without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+package com.jme3.light;
+
+import com.jme3.math.Vector3f;
+import com.jme3.scene.Geometry;
+import com.jme3.scene.Mesh;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Test cases for LightList.
+ */
+public class LightListTest {
+
+    /**
+     * Verify sort() remains correct after the list previously held more lights.
+     */
+    @Test
+    public void testSortAfterRetainedCapacity() {
+        Geometry owner = new Geometry("owner", new Mesh());
+        LightList list = new LightList(owner);
+
+        for (int i = 0; i < 32; i++) {
+            list.add(new PointLight(new Vector3f(100f + i, 0f, 0f)));
+        }
+        list.sort(true);
+        list.clear();
+
+        PointLight far = new PointLight(new Vector3f(10f, 0f, 0f));
+        PointLight near = new PointLight(new Vector3f(1f, 0f, 0f));
+        PointLight middle = new PointLight(new Vector3f(5f, 0f, 0f));
+
+        list.add(far);
+        list.add(near);
+        list.add(middle);
+        list.sort(true);
+
+        Assertions.assertSame(near, list.get(0));
+        Assertions.assertSame(middle, list.get(1));
+        Assertions.assertSame(far, list.get(2));
+    }
+
+    /**
+     * Verify indexed removal preserves order for front, middle, and tail removals.
+     */
+    @Test
+    public void testRemovePreservesOrder() {
+        LightList list = new LightList(new Geometry("owner", new Mesh()));
+        AmbientLight first = new AmbientLight();
+        AmbientLight second = new AmbientLight();
+        AmbientLight third = new AmbientLight();
+        AmbientLight fourth = new AmbientLight();
+
+        list.add(first);
+        list.add(second);
+        list.add(third);
+        list.add(fourth);
+
+        list.remove(0);
+        Assertions.assertEquals(3, list.size());
+        Assertions.assertSame(second, list.get(0));
+        Assertions.assertSame(third, list.get(1));
+        Assertions.assertSame(fourth, list.get(2));
+
+        list.remove(1);
+        Assertions.assertEquals(2, list.size());
+        Assertions.assertSame(second, list.get(0));
+        Assertions.assertSame(fourth, list.get(1));
+
+        list.remove(1);
+        Assertions.assertEquals(1, list.size());
+        Assertions.assertSame(second, list.get(0));
+    }
+
+    /**
+     * Verify unfiltered world-list updates preserve local-then-parent ordering.
+     */
+    @Test
+    public void testUpdateCopiesLocalAndParentLights() {
+        LightList local = new LightList(new Geometry("local", new Mesh()));
+        LightList parent = new LightList(new Geometry("parent", new Mesh()));
+        LightList world = new LightList(new Geometry("world", new Mesh()));
+
+        AmbientLight localFirst = new AmbientLight();
+        AmbientLight localSecond = new AmbientLight();
+        AmbientLight parentFirst = new AmbientLight();
+        AmbientLight parentSecond = new AmbientLight();
+
+        local.add(localFirst);
+        local.add(localSecond);
+        parent.add(parentFirst);
+        parent.add(parentSecond);
+
+        world.update(local, parent);
+
+        Assertions.assertEquals(4, world.size());
+        Assertions.assertSame(localFirst, world.get(0));
+        Assertions.assertSame(localSecond, world.get(1));
+        Assertions.assertSame(parentFirst, world.get(2));
+        Assertions.assertSame(parentSecond, world.get(3));
+    }
+
+    /**
+     * Verify filtered world-list updates keep only accepted local lights.
+     */
+    @Test
+    public void testUpdateFiltersLocalLights() {
+        LightList local = new LightList(new Geometry("local", new Mesh()));
+        LightList parent = new LightList(new Geometry("parent", new Mesh()));
+        LightList world = new LightList(new Geometry("world", new Mesh()));
+
+        AmbientLight keep = new AmbientLight();
+        AmbientLight discard = new AmbientLight();
+        AmbientLight parentLight = new AmbientLight();
+
+        local.add(keep);
+        local.add(discard);
+        parent.add(parentLight);
+
+        world.update(local, parent, light -> light != discard);
+
+        Assertions.assertEquals(2, world.size());
+        Assertions.assertSame(keep, world.get(0));
+        Assertions.assertSame(parentLight, world.get(1));
+    }
+}
