From 9c669547ab875885e95197604e1e8b1c161a0c20 Mon Sep 17 00:00:00 2001 From: tiatin Date: Sat, 25 Jun 2016 14:36:18 +0300 Subject: [PATCH 1/3] Changed iteration over List from for-each to manual iteration. For-Each loop creates Iterator object and uses hasNext and next methods, which are slower, than manual iteration. Also allocating Iterator object increases work for GC. Forum post: https://hub.jmonkeyengine.org/t/iteration-over-list-performance-improvement/36250 See test 9 for more details: http://www.devahead.com/blog/2011/12/coding-for-performance-and-avoiding-garbage-collection-in-android/ --- jme3-core/src/main/java/com/jme3/material/Material.java | 7 ++++++- jme3-core/src/main/java/com/jme3/material/Technique.java | 7 ++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/jme3-core/src/main/java/com/jme3/material/Material.java b/jme3-core/src/main/java/com/jme3/material/Material.java index 74748f578..945b04dae 100644 --- a/jme3-core/src/main/java/com/jme3/material/Material.java +++ b/jme3-core/src/main/java/com/jme3/material/Material.java @@ -750,7 +750,12 @@ public class Material implements CloneableSmartAsset, Cloneable, Savable { } private int applyOverrides(Renderer renderer, Shader shader, List overrides, int unit) { - for (MatParamOverride override : overrides) { + MatParamOverride override; + + // manual iteration is used to avoid iterator allocation and to increase iteration performance + for (int i = 0, listSize = overrides.size(); i < listSize; i++) { + override = overrides.get(i); + VarType type = override.getVarType(); MatParam paramDef = def.getMaterialParam(override.getName()); diff --git a/jme3-core/src/main/java/com/jme3/material/Technique.java b/jme3-core/src/main/java/com/jme3/material/Technique.java index 8ca95b178..0fb7df93d 100644 --- a/jme3-core/src/main/java/com/jme3/material/Technique.java +++ b/jme3-core/src/main/java/com/jme3/material/Technique.java @@ -111,7 +111,12 @@ public final class Technique { } private void applyOverrides(DefineList defineList, List overrides) { - for (MatParamOverride override : overrides) { + MatParamOverride override; + + // manual iteration is used to avoid iterator allocation and to increase iteration performance + for (int i = 0, listSize = overrides.size(); i < listSize; i++) { + override = overrides.get(i); + if (!override.isEnabled()) { continue; } From b524dcd66d950292bf0074c8333c278b17a24359 Mon Sep 17 00:00:00 2001 From: tiatin Date: Sat, 25 Jun 2016 18:15:03 +0300 Subject: [PATCH 2/3] Added iteration using iterator, if List is not ArrayList. Reason for this is that if List is LinkedList, complexity for get(int i) is O(n/4). --- jme3-core/src/main/java/com/jme3/material/Material.java | 6 ++++-- jme3-core/src/main/java/com/jme3/material/Technique.java | 7 +++++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/jme3-core/src/main/java/com/jme3/material/Material.java b/jme3-core/src/main/java/com/jme3/material/Material.java index 945b04dae..cc3b13820 100644 --- a/jme3-core/src/main/java/com/jme3/material/Material.java +++ b/jme3-core/src/main/java/com/jme3/material/Material.java @@ -751,10 +751,12 @@ public class Material implements CloneableSmartAsset, Cloneable, Savable { private int applyOverrides(Renderer renderer, Shader shader, List overrides, int unit) { MatParamOverride override; + boolean isArrayList = overrides instanceof ArrayList; + Iterator iterator = isArrayList ? null : overrides.iterator(); - // manual iteration is used to avoid iterator allocation and to increase iteration performance + // manual iteration is used to avoid iterator allocation and to increase iteration performance in case of ArrayList for (int i = 0, listSize = overrides.size(); i < listSize; i++) { - override = overrides.get(i); + override = isArrayList ? overrides.get(i) : iterator.next(); VarType type = override.getVarType(); diff --git a/jme3-core/src/main/java/com/jme3/material/Technique.java b/jme3-core/src/main/java/com/jme3/material/Technique.java index 0fb7df93d..592629975 100644 --- a/jme3-core/src/main/java/com/jme3/material/Technique.java +++ b/jme3-core/src/main/java/com/jme3/material/Technique.java @@ -44,6 +44,7 @@ import com.jme3.shader.VarType; import com.jme3.util.ListMap; import java.util.ArrayList; import java.util.EnumSet; +import java.util.Iterator; import java.util.List; /** @@ -112,10 +113,12 @@ public final class Technique { private void applyOverrides(DefineList defineList, List overrides) { MatParamOverride override; + boolean isArrayList = overrides instanceof ArrayList; + Iterator iterator = isArrayList ? null : overrides.iterator(); - // manual iteration is used to avoid iterator allocation and to increase iteration performance + // manual iteration is used to avoid iterator allocation and to increase iteration performance in case of ArrayList for (int i = 0, listSize = overrides.size(); i < listSize; i++) { - override = overrides.get(i); + override = isArrayList ? overrides.get(i) : iterator.next(); if (!override.isEnabled()) { continue; From 8e8186de0acffa372e5ba67afda456a2ce7fca99 Mon Sep 17 00:00:00 2001 From: tiatin Date: Sun, 26 Jun 2016 12:31:34 +0300 Subject: [PATCH 3/3] Changed overrides from ArrayList to SafeArrayList for GC and iteration performance reasons. Fixed bug in SafeArrayList.equals(). --- .../main/java/com/jme3/material/Material.java | 19 +++++-------- .../java/com/jme3/material/Technique.java | 21 ++++---------- .../java/com/jme3/renderer/RenderManager.java | 4 +-- .../src/main/java/com/jme3/scene/Spatial.java | 28 ++++++++++--------- .../java/com/jme3/util/SafeArrayList.java | 2 +- 5 files changed, 31 insertions(+), 43 deletions(-) diff --git a/jme3-core/src/main/java/com/jme3/material/Material.java b/jme3-core/src/main/java/com/jme3/material/Material.java index cc3b13820..b8d002932 100644 --- a/jme3-core/src/main/java/com/jme3/material/Material.java +++ b/jme3-core/src/main/java/com/jme3/material/Material.java @@ -35,7 +35,7 @@ import com.jme3.asset.AssetKey; import com.jme3.asset.AssetManager; import com.jme3.asset.CloneableSmartAsset; import com.jme3.export.*; -import com.jme3.light.*; +import com.jme3.light.LightList; import com.jme3.material.RenderState.BlendMode; import com.jme3.material.RenderState.FaceCullMode; import com.jme3.material.TechniqueDef.LightMode; @@ -54,6 +54,8 @@ import com.jme3.texture.Image; import com.jme3.texture.Texture; import com.jme3.texture.image.ColorSpace; import com.jme3.util.ListMap; +import com.jme3.util.SafeArrayList; + import java.io.IOException; import java.util.*; import java.util.logging.Level; @@ -749,15 +751,8 @@ public class Material implements CloneableSmartAsset, Cloneable, Savable { sortingId = -1; } - private int applyOverrides(Renderer renderer, Shader shader, List overrides, int unit) { - MatParamOverride override; - boolean isArrayList = overrides instanceof ArrayList; - Iterator iterator = isArrayList ? null : overrides.iterator(); - - // manual iteration is used to avoid iterator allocation and to increase iteration performance in case of ArrayList - for (int i = 0, listSize = overrides.size(); i < listSize; i++) { - override = isArrayList ? overrides.get(i) : iterator.next(); - + private int applyOverrides(Renderer renderer, Shader shader, SafeArrayList overrides, int unit) { + for (MatParamOverride override : overrides.getArray()) { VarType type = override.getVarType(); MatParam paramDef = def.getMaterialParam(override.getName()); @@ -784,7 +779,7 @@ public class Material implements CloneableSmartAsset, Cloneable, Savable { } private void updateShaderMaterialParameters(Renderer renderer, Shader shader, - List worldOverrides, List forcedOverrides) { + SafeArrayList worldOverrides, SafeArrayList forcedOverrides) { int unit = 0; if (worldOverrides != null) { @@ -952,7 +947,7 @@ public class Material implements CloneableSmartAsset, Cloneable, Savable { updateRenderState(renderManager, renderer, techniqueDef); // Get world overrides - List overrides = geometry.getWorldMatParamOverrides(); + SafeArrayList overrides = geometry.getWorldMatParamOverrides(); // Select shader to use Shader shader = technique.makeCurrent(renderManager, overrides, renderManager.getForcedMatParams(), lights, rendererCaps); diff --git a/jme3-core/src/main/java/com/jme3/material/Technique.java b/jme3-core/src/main/java/com/jme3/material/Technique.java index 592629975..7128747c9 100644 --- a/jme3-core/src/main/java/com/jme3/material/Technique.java +++ b/jme3-core/src/main/java/com/jme3/material/Technique.java @@ -31,10 +31,10 @@ */ package com.jme3.material; -import com.jme3.material.logic.TechniqueDefLogic; import com.jme3.asset.AssetManager; import com.jme3.light.LightList; import com.jme3.material.TechniqueDef.LightMode; +import com.jme3.material.logic.TechniqueDefLogic; import com.jme3.renderer.Caps; import com.jme3.renderer.RenderManager; import com.jme3.scene.Geometry; @@ -42,10 +42,8 @@ import com.jme3.shader.DefineList; import com.jme3.shader.Shader; import com.jme3.shader.VarType; import com.jme3.util.ListMap; -import java.util.ArrayList; +import com.jme3.util.SafeArrayList; import java.util.EnumSet; -import java.util.Iterator; -import java.util.List; /** * Represents a technique instance. @@ -111,15 +109,8 @@ public final class Technique { } } - private void applyOverrides(DefineList defineList, List overrides) { - MatParamOverride override; - boolean isArrayList = overrides instanceof ArrayList; - Iterator iterator = isArrayList ? null : overrides.iterator(); - - // manual iteration is used to avoid iterator allocation and to increase iteration performance in case of ArrayList - for (int i = 0, listSize = overrides.size(); i < listSize; i++) { - override = isArrayList ? overrides.get(i) : iterator.next(); - + private void applyOverrides(DefineList defineList, SafeArrayList overrides) { + for (MatParamOverride override : overrides.getArray()) { if (!override.isEnabled()) { continue; } @@ -142,8 +133,8 @@ public final class Technique { * @param rendererCaps The renderer capabilities which the shader should support. * @return A compatible shader. */ - Shader makeCurrent(RenderManager renderManager, List worldOverrides, - List forcedOverrides, + Shader makeCurrent(RenderManager renderManager, SafeArrayList worldOverrides, + SafeArrayList forcedOverrides, LightList lights, EnumSet rendererCaps) { TechniqueDefLogic logic = def.getLogic(); AssetManager assetManager = owner.getMaterialDef().getAssetManager(); diff --git a/jme3-core/src/main/java/com/jme3/renderer/RenderManager.java b/jme3-core/src/main/java/com/jme3/renderer/RenderManager.java index 50cc207f7..b07b95092 100644 --- a/jme3-core/src/main/java/com/jme3/renderer/RenderManager.java +++ b/jme3-core/src/main/java/com/jme3/renderer/RenderManager.java @@ -83,7 +83,7 @@ public class RenderManager { private Material forcedMaterial = null; private String forcedTechnique = null; private RenderState forcedRenderState = null; - private final List forcedOverrides = new ArrayList<>(); + private final SafeArrayList forcedOverrides = new SafeArrayList<>(MatParamOverride.class); private int viewX, viewY, viewWidth, viewHeight; private final Matrix4f orthoMatrix = new Matrix4f(); private final LightList filteredLightList = new LightList(null); @@ -462,7 +462,7 @@ public class RenderManager { * * @return The forced material parameters. */ - public List getForcedMatParams() { + public SafeArrayList getForcedMatParams() { return forcedOverrides; } diff --git a/jme3-core/src/main/java/com/jme3/scene/Spatial.java b/jme3-core/src/main/java/com/jme3/scene/Spatial.java index d0369ca3c..afffcd647 100644 --- a/jme3-core/src/main/java/com/jme3/scene/Spatial.java +++ b/jme3-core/src/main/java/com/jme3/scene/Spatial.java @@ -138,8 +138,8 @@ public abstract class Spatial implements Savable, Cloneable, Collidable, Cloneab protected LightList localLights; protected transient LightList worldLights; - protected ArrayList localOverrides; - protected ArrayList worldOverrides; + protected SafeArrayList localOverrides; + protected SafeArrayList worldOverrides; /** * This spatial's name. @@ -207,8 +207,8 @@ public abstract class Spatial implements Savable, Cloneable, Collidable, Cloneab localLights = new LightList(this); worldLights = new LightList(this); - localOverrides = new ArrayList<>(); - worldOverrides = new ArrayList<>(); + localOverrides = new SafeArrayList<>(MatParamOverride.class); + worldOverrides = new SafeArrayList<>(MatParamOverride.class); refreshFlags |= RF_BOUND; } @@ -432,7 +432,7 @@ public abstract class Spatial implements Savable, Cloneable, Collidable, Cloneab * * @return The list of local material parameter overrides. */ - public List getLocalMatParamOverrides() { + public SafeArrayList getLocalMatParamOverrides() { return localOverrides; } @@ -446,7 +446,7 @@ public abstract class Spatial implements Savable, Cloneable, Collidable, Cloneab * * @return The list of world material parameter overrides. */ - public List getWorldMatParamOverrides() { + public SafeArrayList getWorldMatParamOverrides() { return worldOverrides; } @@ -1384,8 +1384,8 @@ public abstract class Spatial implements Savable, Cloneable, Collidable, Cloneab clone.localLights.setOwner(clone); clone.worldLights.setOwner(clone); - clone.worldOverrides = new ArrayList(); - clone.localOverrides = new ArrayList(); + clone.worldOverrides = new SafeArrayList<>(MatParamOverride.class); + clone.localOverrides = new SafeArrayList<>(MatParamOverride.class); for (MatParamOverride override : localOverrides) { clone.localOverrides.add((MatParamOverride) override.clone()); @@ -1598,7 +1598,7 @@ public abstract class Spatial implements Savable, Cloneable, Collidable, Cloneab capsule.write(shadowMode, "shadow_mode", ShadowMode.Inherit); capsule.write(localTransform, "transform", Transform.IDENTITY); capsule.write(localLights, "lights", null); - capsule.writeSavableArrayList(localOverrides, "overrides", null); + capsule.writeSavableArrayList(new ArrayList(localOverrides), "overrides", null); // Shallow clone the controls array to convert its type. capsule.writeSavableArrayList(new ArrayList(controls), "controlsList", null); @@ -1622,11 +1622,13 @@ public abstract class Spatial implements Savable, Cloneable, Collidable, Cloneab localLights = (LightList) ic.readSavable("lights", null); localLights.setOwner(this); - localOverrides = ic.readSavableArrayList("overrides", null); - if (localOverrides == null) { - localOverrides = new ArrayList<>(); + ArrayList localOverridesList = ic.readSavableArrayList("overrides", null); + if (localOverridesList == null) { + localOverrides = new SafeArrayList<>(MatParamOverride.class); + } else { + localOverrides = new SafeArrayList(MatParamOverride.class, localOverridesList); } - worldOverrides = new ArrayList<>(); + worldOverrides = new SafeArrayList<>(MatParamOverride.class); //changed for backward compatibility with j3o files generated before the AnimControl/SkeletonControl split //the AnimControl creates the SkeletonControl for old files and add it to the spatial. diff --git a/jme3-core/src/main/java/com/jme3/util/SafeArrayList.java b/jme3-core/src/main/java/com/jme3/util/SafeArrayList.java index a0657c753..30593cd4b 100644 --- a/jme3-core/src/main/java/com/jme3/util/SafeArrayList.java +++ b/jme3-core/src/main/java/com/jme3/util/SafeArrayList.java @@ -258,7 +258,7 @@ public class SafeArrayList implements List, Cloneable { if( o1 == null || !o1.equals(o2) ) return false; } - return !(i1.hasNext() || !i2.hasNext()); + return !(i1.hasNext() || i2.hasNext()); } public int hashCode() {