Skip to content

Commit 5906442

Browse files
8Keep8Keep
committed
Optimize LightList hot path copies
1 parent cc0750d commit 5906442

2 files changed

Lines changed: 189 additions & 22 deletions

File tree

‎jme3-core/src/main/java/com/jme3/light/LightList.java‎

Lines changed: 33 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ public final class LightList implements Iterable<Light>, Savable, Cloneable, Jme
5151
private Light[] list, tlist;
5252
private float[] distToOwner;
5353
private int listSize;
54+
private int tlistSize;
5455
private Spatial owner;
5556

5657
private static final int DEFAULT_SIZE = 1;
@@ -136,8 +137,9 @@ public void remove(int index) {
136137
return;
137138
}
138139

139-
for (int i = index; i < listSize; i++) {
140-
list[i] = list[i+1];
140+
int copyLength = listSize - index;
141+
if (copyLength > 0) {
142+
System.arraycopy(list, index + 1, list, index, copyLength);
141143
}
142144
list[listSize] = null;
143145
}
@@ -182,11 +184,11 @@ public void clear() {
182184
if (listSize == 0)
183185
return;
184186

185-
for (int i = 0; i < listSize; i++)
186-
list[i] = null;
187+
Arrays.fill(list, 0, listSize, null);
187188

188189
if (tlist != null)
189-
Arrays.fill(tlist, null);
190+
Arrays.fill(tlist, 0, tlistSize, null);
191+
tlistSize = 0;
190192

191193
listSize = 0;
192194
}
@@ -205,11 +207,15 @@ public void clear() {
205207
public void sort(boolean transformChanged) {
206208
if (listSize > 1) {
207209
// resize or populate our temporary array as necessary
208-
if (tlist == null || tlist.length != list.length) {
209-
tlist = list.clone();
210+
if (tlist == null || tlist.length < listSize) {
211+
tlist = new Light[listSize];
210212
} else {
211-
System.arraycopy(list, 0, tlist, 0, list.length);
213+
if (tlistSize > listSize) {
214+
Arrays.fill(tlist, listSize, tlistSize, null);
215+
}
212216
}
217+
System.arraycopy(list, 0, tlist, 0, listSize);
218+
tlistSize = listSize;
213219

214220
if (transformChanged) {
215221
// check distance of each light
@@ -249,31 +255,36 @@ public void update(LightList local, LightList parent, Predicate<Light> filter) {
249255
// using the arguments
250256
clear();
251257

252-
while (list.length <= local.listSize) {
258+
int requiredSize = local.listSize + (parent == null ? 0 : parent.listSize);
259+
while (list.length < requiredSize) {
253260
doubleSize();
254261
}
255262

256-
int localListSize = 0;
257-
for(int i=0;i<local.listSize;i++){
258-
Light l = local.list[i];
259-
if (filter != null && !filter.test(l)) continue;
260-
list[localListSize] = l;
261-
distToOwner[localListSize] = Float.NEGATIVE_INFINITY;
262-
localListSize++;
263+
int localListSize;
264+
if (filter == null) {
265+
localListSize = local.listSize;
266+
System.arraycopy(local.list, 0, list, 0, localListSize);
267+
Arrays.fill(distToOwner, 0, localListSize, Float.NEGATIVE_INFINITY);
268+
} else {
269+
localListSize = 0;
270+
for(int i=0;i<local.listSize;i++){
271+
Light l = local.list[i];
272+
if (!filter.test(l)) continue;
273+
list[localListSize] = l;
274+
distToOwner[localListSize] = Float.NEGATIVE_INFINITY;
275+
localListSize++;
276+
}
263277
}
264278

265279
// if the spatial has a parent node, add the lights
266280
// from the parent list as well
267281
if (parent != null) {
268282
int sz = localListSize + parent.listSize;
269-
while (list.length <= sz)
283+
while (list.length < sz)
270284
doubleSize();
271285

272-
for (int i = 0; i < parent.listSize; i++) {
273-
int p = i + localListSize;
274-
list[p] = parent.list[i];
275-
distToOwner[p] = Float.NEGATIVE_INFINITY;
276-
}
286+
System.arraycopy(parent.list, 0, list, localListSize, parent.listSize);
287+
Arrays.fill(distToOwner, localListSize, sz, Float.NEGATIVE_INFINITY);
277288

278289
listSize = localListSize + parent.listSize;
279290
} else {

‎jme3-core/src/test/java/com/jme3/light/LightListTest.java‎

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
/*
2+
* Copyright (c) 2026 jMonkeyEngine
3+
* All rights reserved.
4+
*
5+
* Redistribution and use in source and binary forms, with or without
6+
* modification, are permitted provided that the following conditions are
7+
* met:
8+
*
9+
* * Redistributions of source code must retain the above copyright
10+
* notice, this list of conditions and the following disclaimer.
11+
*
12+
* * Redistributions in binary form must reproduce the above copyright
13+
* notice, this list of conditions and the following disclaimer in the
14+
* documentation and/or other materials provided with the distribution.
15+
*
16+
* * Neither the name of 'jMonkeyEngine' nor the names of its contributors
17+
* may be used to endorse or promote products derived from this software
18+
* without specific prior written permission.
19+
*
20+
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
21+
* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
22+
* TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
23+
* PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
24+
* CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
25+
* EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
26+
* PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
27+
* PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
28+
* LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
29+
* NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
30+
* SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
31+
*/
32+
package com.jme3.light;
33+
34+
import com.jme3.math.Vector3f;
35+
import com.jme3.scene.Geometry;
36+
import com.jme3.scene.Mesh;
37+
import org.junit.jupiter.api.Assertions;
38+
import org.junit.jupiter.api.Test;
39+
40+
/**
41+
* Test cases for LightList.
42+
*/
43+
public class LightListTest {
44+
45+
/**
46+
* Verify sort() remains correct after the list previously held more lights.
47+
*/
48+
@Test
49+
public void testSortAfterRetainedCapacity() {
50+
Geometry owner = new Geometry("owner", new Mesh());
51+
LightList list = new LightList(owner);
52+
53+
for (int i = 0; i < 32; i++) {
54+
list.add(new PointLight(new Vector3f(100f + i, 0f, 0f)));
55+
}
56+
list.sort(true);
57+
list.clear();
58+
59+
PointLight far = new PointLight(new Vector3f(10f, 0f, 0f));
60+
PointLight near = new PointLight(new Vector3f(1f, 0f, 0f));
61+
PointLight middle = new PointLight(new Vector3f(5f, 0f, 0f));
62+
63+
list.add(far);
64+
list.add(near);
65+
list.add(middle);
66+
list.sort(true);
67+
68+
Assertions.assertSame(near, list.get(0));
69+
Assertions.assertSame(middle, list.get(1));
70+
Assertions.assertSame(far, list.get(2));
71+
}
72+
73+
/**
74+
* Verify indexed removal preserves order for front, middle, and tail removals.
75+
*/
76+
@Test
77+
public void testRemovePreservesOrder() {
78+
LightList list = new LightList(new Geometry("owner", new Mesh()));
79+
AmbientLight first = new AmbientLight();
80+
AmbientLight second = new AmbientLight();
81+
AmbientLight third = new AmbientLight();
82+
AmbientLight fourth = new AmbientLight();
83+
84+
list.add(first);
85+
list.add(second);
86+
list.add(third);
87+
list.add(fourth);
88+
89+
list.remove(0);
90+
Assertions.assertEquals(3, list.size());
91+
Assertions.assertSame(second, list.get(0));
92+
Assertions.assertSame(third, list.get(1));
93+
Assertions.assertSame(fourth, list.get(2));
94+
95+
list.remove(1);
96+
Assertions.assertEquals(2, list.size());
97+
Assertions.assertSame(second, list.get(0));
98+
Assertions.assertSame(fourth, list.get(1));
99+
100+
list.remove(1);
101+
Assertions.assertEquals(1, list.size());
102+
Assertions.assertSame(second, list.get(0));
103+
}
104+
105+
/**
106+
* Verify unfiltered world-list updates preserve local-then-parent ordering.
107+
*/
108+
@Test
109+
public void testUpdateCopiesLocalAndParentLights() {
110+
LightList local = new LightList(new Geometry("local", new Mesh()));
111+
LightList parent = new LightList(new Geometry("parent", new Mesh()));
112+
LightList world = new LightList(new Geometry("world", new Mesh()));
113+
114+
AmbientLight localFirst = new AmbientLight();
115+
AmbientLight localSecond = new AmbientLight();
116+
AmbientLight parentFirst = new AmbientLight();
117+
AmbientLight parentSecond = new AmbientLight();
118+
119+
local.add(localFirst);
120+
local.add(localSecond);
121+
parent.add(parentFirst);
122+
parent.add(parentSecond);
123+
124+
world.update(local, parent);
125+
126+
Assertions.assertEquals(4, world.size());
127+
Assertions.assertSame(localFirst, world.get(0));
128+
Assertions.assertSame(localSecond, world.get(1));
129+
Assertions.assertSame(parentFirst, world.get(2));
130+
Assertions.assertSame(parentSecond, world.get(3));
131+
}
132+
133+
/**
134+
* Verify filtered world-list updates keep only accepted local lights.
135+
*/
136+
@Test
137+
public void testUpdateFiltersLocalLights() {
138+
LightList local = new LightList(new Geometry("local", new Mesh()));
139+
LightList parent = new LightList(new Geometry("parent", new Mesh()));
140+
LightList world = new LightList(new Geometry("world", new Mesh()));
141+
142+
AmbientLight keep = new AmbientLight();
143+
AmbientLight discard = new AmbientLight();
144+
AmbientLight parentLight = new AmbientLight();
145+
146+
local.add(keep);
147+
local.add(discard);
148+
parent.add(parentLight);
149+
150+
world.update(local, parent, light -> light != discard);
151+
152+
Assertions.assertEquals(2, world.size());
153+
Assertions.assertSame(keep, world.get(0));
154+
Assertions.assertSame(parentLight, world.get(1));
155+
}
156+
}

0 commit comments

Comments
 (0)