Skip to content

Commit

Permalink
Fix: Memory consumption
Browse files Browse the repository at this point in the history 
Observing memory consumption showed:
    1 - 'traceLock' debug stack traces (GLContextLock)
    2 - massive Iterator usage

(1) is fixed, ie only enabled in DEBUG mode, like we have done in RecursiveLock before

(2) Using an Iterator on ArrayLists with a low element count < 100,
    as it is usual in our use cases, is observed not to be faster
    than accessing the elements via an index (-> TestIteratorIndexCORE.java ).
    On the contrary, the index implementation was a bit faster.
    Further more, these Iterators were massively used on the fly during animation,
    hence their memory managment even impacts fluent processing/animation.
    Recoded all animation related (display, surfaceUpdated, ..) loops using an index.
  • Loading branch information
@sgothel
sgothel committed Oct 14, 2010
1 parent 29999cf commit 6ced17f
Show file tree
Hide file tree
Showing 6 changed files with 241 additions and 64 deletions.
19 changes: 14 additions & 5 deletions src/jogl/classes/com/jogamp/opengl/impl/GLContextLock.java
View file
Expand Up @@ -50,11 +50,13 @@
be raised. */

public class GLContextLock {
protected static final boolean DEBUG = GLContextImpl.DEBUG;

static class SyncData {
boolean failFastMode = true;
Thread owner = null;
int waiters = 0;
Exception lockedStack = null;
Exception lockedStack = null; // only enabled if DEBUG
}
private SyncData sdata = new SyncData(); // synchronized (flow/mem) mutable access

Expand All @@ -66,12 +68,16 @@ public final void lock() throws GLException {
Thread current = Thread.currentThread();
if (sdata.owner == null) {
sdata.owner = current;
sdata.lockedStack = new Exception("Previously made current (1) by "+sdata.owner+", lock: "+this);
if(DEBUG) {
sdata.lockedStack = new Exception("Error: Previously made current (1) by "+sdata.owner+", lock: "+this);
}
} else if (sdata.owner != current) {
while (sdata.owner != null) {
if (sdata.failFastMode) {
sdata.lockedStack.printStackTrace();
throw new GLException("Attempt to make context current on thread " + current +
if(null!=sdata.lockedStack) {
sdata.lockedStack.printStackTrace();
}
throw new GLException("Error: Attempt to make context current on thread " + current +
" which is already current on thread " + sdata.owner);
} else {
try {
Expand All @@ -85,7 +91,9 @@ public final void lock() throws GLException {
}
}
sdata.owner = current;
sdata.lockedStack = new Exception("Previously made current (2) by "+sdata.owner+", lock: "+this);
if(DEBUG) {
sdata.lockedStack = new Exception("Previously made current (2) by "+sdata.owner+", lock: "+this);
}
} else {
throw new GLException("Attempt to make the same context current twice on thread " + current);
}
Expand Down Expand Up @@ -139,6 +147,7 @@ public final boolean hasWaiters() {
}
}

/** holding the owners stack trace when lock is acquired and DEBUG is true */
public final Exception getLockedStack() {
synchronized(sdata) {
return sdata.lockedStack;
Expand Down
51 changes: 32 additions & 19 deletions src/jogl/classes/com/jogamp/opengl/impl/GLDrawableHelper.java
View file
Expand Up @@ -50,15 +50,29 @@ public class GLDrawableHelper {
protected static final boolean DEBUG = GLDrawableImpl.DEBUG;
private static final boolean VERBOSE = Debug.verbose();
private Object listenersLock = new Object();
private List listeners = new ArrayList();
private volatile boolean listenersIter = false; // avoid java.util.ConcurrentModificationException
private Set listenersToBeInit = new HashSet();
private boolean autoSwapBufferMode = true;
private List listeners;
private volatile boolean listenersIter; // avoid java.util.ConcurrentModificationException
private Set listenersToBeInit;
private boolean autoSwapBufferMode;
private Object glRunnablesLock = new Object();
private ArrayList glRunnables = new ArrayList(); // one shot GL tasks
private GLAnimatorControl animatorCtrl = null; // default
private ArrayList glRunnables;
private GLAnimatorControl animatorCtrl;

public GLDrawableHelper() {
reset();
}

public void reset() {
synchronized(listenersLock) {
listeners = new ArrayList();
listenersIter = false;
listenersToBeInit = new HashSet();
}
autoSwapBufferMode = true;
synchronized(glRunnablesLock) {
glRunnables = new ArrayList();
}
animatorCtrl = null;
}

public String toString() {
Expand All @@ -67,8 +81,8 @@ public String toString() {
synchronized(listenersLock) {
sb.append("GLEventListeners num "+listeners.size()+" [");
listenersIter = true;
for (Iterator iter = listeners.iterator(); iter.hasNext(); ) {
Object l = iter.next();
for (int i=0; i < listeners.size(); i++) {
Object l = listeners.get(i);
sb.append(l);
sb.append("[init ");
sb.append( !listenersToBeInit.contains(l) );
Expand Down Expand Up @@ -120,8 +134,8 @@ public void removeGLEventListener(GLEventListener listener) {
public void dispose(GLAutoDrawable drawable) {
synchronized(listenersLock) {
listenersIter = true;
for (Iterator iter = listeners.iterator(); iter.hasNext(); ) {
GLEventListener listener = (GLEventListener) iter.next() ;
for (int i=0; i < listeners.size(); i++) {
GLEventListener listener = (GLEventListener) listeners.get(i) ;
listener.dispose(drawable);
listenersToBeInit.add(listener);
}
Expand All @@ -143,8 +157,8 @@ private final boolean init(GLEventListener l, GLAutoDrawable drawable, boolean s
public void init(GLAutoDrawable drawable) {
synchronized(listenersLock) {
listenersIter = true;
for (Iterator iter = listeners.iterator(); iter.hasNext(); ) {
GLEventListener listener = (GLEventListener) iter.next() ;
for (int i=0; i < listeners.size(); i++) {
GLEventListener listener = (GLEventListener) listeners.get(i) ;
if ( ! init( listener, drawable, false ) ) {
throw new GLException("GLEventListener "+listener+" already initialized: "+drawable);
}
Expand All @@ -156,8 +170,8 @@ public void init(GLAutoDrawable drawable) {
public void display(GLAutoDrawable drawable) {
synchronized(listenersLock) {
listenersIter = true;
for (Iterator iter = listeners.iterator(); iter.hasNext(); ) {
GLEventListener listener = (GLEventListener) iter.next() ;
for (int i=0; i < listeners.size(); i++) {
GLEventListener listener = (GLEventListener) listeners.get(i) ;
// GLEventListener may need to be init,
// in case this one is added after the realization of the GLAutoDrawable
init( listener, drawable, true ) ;
Expand All @@ -179,9 +193,8 @@ private final void reshape(GLEventListener listener, GLAutoDrawable drawable,
public void reshape(GLAutoDrawable drawable, int x, int y, int width, int height) {
synchronized(listenersLock) {
listenersIter = true;
int i=0;
for (Iterator iter = listeners.iterator(); iter.hasNext(); i++) {
reshape((GLEventListener) iter.next(), drawable, x, y, width, height, 0==i);
for (int i=0; i < listeners.size(); i++) {
reshape((GLEventListener) listeners.get(i), drawable, x, y, width, height, 0==i);
}
listenersIter = false;
}
Expand All @@ -198,8 +211,8 @@ private void execGLRunnables(GLAutoDrawable drawable) {
}
}
if(null!=_glRunnables) {
for (Iterator iter = _glRunnables.iterator(); iter.hasNext(); ) {
((GLRunnable) iter.next()).run(drawable);
for (int i=0; i < _glRunnables.size(); i++) {
((GLRunnable) _glRunnables.get(i)).run(drawable);
}
}
}
Expand Down
43 changes: 25 additions & 18 deletions src/jogl/classes/com/jogamp/opengl/util/AWTAnimatorImpl.java
View file
Expand Up @@ -55,29 +55,36 @@ class AWTAnimatorImpl extends AnimatorImpl {
public void display(AnimatorBase animator,
boolean ignoreExceptions,
boolean printExceptions) {
Iterator iter = animator.drawableIterator();
while (animator.isAnimating() && !animator.getShouldStop() && !animator.getShouldPause() && iter.hasNext()) {
GLAutoDrawable drawable = (GLAutoDrawable) iter.next();
if (drawable instanceof JComponent) {
// Lightweight components need a more efficient drawing
// scheme than simply forcing repainting of each one in
// turn since drawing one can force another one to be
// drawn in turn
lightweights.add(drawable);
} else {
try {
drawable.display();
} catch (RuntimeException e) {
if (ignoreExceptions) {
if (printExceptions) {
e.printStackTrace();
List drawables = animator.acquireDrawables();
try {
for (int i=0;
animator.isAnimating() && !animator.getShouldStop() && !animator.getShouldPause() && i<drawables.size();
i++) {
GLAutoDrawable drawable = (GLAutoDrawable) drawables.get(i);
if (drawable instanceof JComponent) {
// Lightweight components need a more efficient drawing
// scheme than simply forcing repainting of each one in
// turn since drawing one can force another one to be
// drawn in turn
lightweights.add(drawable);
} else {
try {
drawable.display();
} catch (RuntimeException e) {
if (ignoreExceptions) {
if (printExceptions) {
e.printStackTrace();
}
} else {
throw(e);
}
} else {
throw(e);
}
}
}
} finally {
animator.releaseDrawables();
}

if (lightweights.size() > 0) {
try {
SwingUtilities.invokeAndWait(drawWithRepaintManagerRunnable);
Expand Down
36 changes: 25 additions & 11 deletions src/jogl/classes/com/jogamp/opengl/util/AnimatorBase.java
View file
Expand Up @@ -28,9 +28,11 @@

package com.jogamp.opengl.util;

import com.jogamp.common.util.locks.RecursiveLock;
import com.jogamp.opengl.impl.Debug;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
import javax.media.opengl.GLAnimatorControl;
import javax.media.opengl.GLAutoDrawable;
import javax.media.opengl.GLProfile;
Expand All @@ -43,7 +45,8 @@ public abstract class AnimatorBase implements GLAnimatorControl {

private static int animatorCount = 0;

protected volatile ArrayList/*<GLAutoDrawable>*/ drawables = new ArrayList();
protected ArrayList/*<GLAutoDrawable>*/ drawables = new ArrayList();
protected RecursiveLock drawablesLock = new RecursiveLock();
protected AnimatorImpl impl;
protected String baseName;
protected Thread thread;
Expand Down Expand Up @@ -75,18 +78,24 @@ public AnimatorBase() {
protected abstract String getBaseName(String prefix);

public synchronized void add(GLAutoDrawable drawable) {
ArrayList newList = (ArrayList) drawables.clone();
newList.add(drawable);
drawables = newList;
drawable.setAnimator(this);
drawablesLock.lock();
try {
drawables.add(drawable);
drawable.setAnimator(this);
} finally {
drawablesLock.unlock();
}
notifyAll();
}

public synchronized void remove(GLAutoDrawable drawable) {
ArrayList newList = (ArrayList) drawables.clone();
newList.remove(drawable);
drawables = newList;
drawable.setAnimator(null);
drawablesLock.lock();
try {
drawables.remove(drawable);
drawable.setAnimator(null);
} finally {
drawablesLock.unlock();
}
notifyAll();
}

Expand All @@ -101,8 +110,13 @@ protected void display() {
totalFrames++;
}

public Iterator drawableIterator() {
return drawables.iterator();
public List acquireDrawables() {
drawablesLock.lock();
return drawables;
}

public void releaseDrawables() {
drawablesLock.unlock();
}

public long getCurrentTime() {
Expand Down
28 changes: 17 additions & 11 deletions src/jogl/classes/com/jogamp/opengl/util/AnimatorImpl.java
View file
Expand Up @@ -44,20 +44,26 @@ class AnimatorImpl {
public void display(AnimatorBase animator,
boolean ignoreExceptions,
boolean printExceptions) {
Iterator iter = animator.drawableIterator();
while (animator.isAnimating() && !animator.getShouldStop() && !animator.getShouldPause() && iter.hasNext()) {
GLAutoDrawable drawable = (GLAutoDrawable) iter.next();
try {
drawable.display();
} catch (RuntimeException e) {
if (ignoreExceptions) {
if (printExceptions) {
e.printStackTrace();
List drawables = animator.acquireDrawables();
try {
for (int i=0;
animator.isAnimating() && !animator.getShouldStop() && !animator.getShouldPause() && i<drawables.size();
i++) {
GLAutoDrawable drawable = (GLAutoDrawable) drawables.get(i);
try {
drawable.display();
} catch (RuntimeException e) {
if (ignoreExceptions) {
if (printExceptions) {
e.printStackTrace();
}
} else {
throw(e);
}
} else {
throw(e);
}
}
} finally {
animator.releaseDrawables();
}
}

Expand Down

0 comments on commit 6ced17f

Please sign in to comment.