You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pivot.apache.org by rw...@apache.org on 2013/02/20 01:13:33 UTC
svn commit: r1447977 - in /pivot/branches/2.0.x:
core/src/org/apache/pivot/util/ListenerList.java
wtk/src/org/apache/pivot/wtk/content/ButtonData.java
wtk/src/org/apache/pivot/wtk/media/Image.java
Author: rwhitcomb
Date: Wed Feb 20 00:13:32 2013
New Revision: 1447977
URL: http://svn.apache.org/r1447977
Log:
Part of PIVOT-861: Lingering memory leak of ListenerList$Node objects when continuosly
creating and destroying windows or buttons (from BXMLSerializer).
It seemed like the basic problem was every time you use a ButtonRenderer, there is a
call to ImageView.setIcon which takes the old image out of the ImageViewSkin listener
list and puts the new one in, ending up creating a huge number of ListenerList$Node
objects for every add to these lists.
So, I rewrote the whole of ListenerList to use a small array for these lists instead
of a real linked list. Empirically the lists don't usually grow very big, so the
default size of 5 works well except for a handful of cases, and then the largest
seen was 25.
The result is that I don't see any long-term memory leak at all in the test applications
and the overall memory thrashing seems less because we're creating far fewer of these
little Node objects (just to add and remove a reference in a small list).
Also reverted the last changes to ButtonData.java and Image.java to take out the
earlier workaround that wasn't quite working.
Modified:
pivot/branches/2.0.x/core/src/org/apache/pivot/util/ListenerList.java
pivot/branches/2.0.x/wtk/src/org/apache/pivot/wtk/content/ButtonData.java
pivot/branches/2.0.x/wtk/src/org/apache/pivot/wtk/media/Image.java
Modified: pivot/branches/2.0.x/core/src/org/apache/pivot/util/ListenerList.java
URL: http://svn.apache.org/viewvc/pivot/branches/2.0.x/core/src/org/apache/pivot/util/ListenerList.java?rev=1447977&r1=1447976&r2=1447977&view=diff
==============================================================================
--- pivot/branches/2.0.x/core/src/org/apache/pivot/util/ListenerList.java (original)
+++ pivot/branches/2.0.x/core/src/org/apache/pivot/util/ListenerList.java Wed Feb 20 00:13:32 2013
@@ -16,6 +16,7 @@
*/
package org.apache.pivot.util;
+import java.util.Arrays;
import java.util.Iterator;
import java.util.NoSuchElementException;
@@ -28,42 +29,27 @@ import java.util.NoSuchElementException;
* safety during iteration.
*/
public abstract class ListenerList<T> implements Iterable<T> {
- // Node containing a listener in the list
- private class Node {
- private Node previous;
- private Node next;
- private T listener;
- public Node(Node previous, Node next, T listener) {
- this.previous = previous;
- this.next = next;
- this.listener = listener;
- }
- }
-
- // Node iterator
+ // Iterator through the current array of elements
private class NodeIterator implements Iterator<T> {
- private Node node;
+ private int index;
public NodeIterator() {
- this.node = first;
+ this.index = 0;
}
@Override
public boolean hasNext() {
- return (node != null);
+ return (index < last);
}
@Override
public T next() {
- if (node == null) {
+ if (index >= last) {
throw new NoSuchElementException();
}
- T listener = node.listener;
- node = node.next;
-
- return listener;
+ return list[index++];
}
@Override
@@ -72,9 +58,15 @@ public abstract class ListenerList<T> im
}
}
- // First node in the list (we don't maintain a reference to the last
- // node, since we need to walk the list looking for duplicates on add)
- private Node first = null;
+ private static final int DEFAULT_SIZE = 5;
+
+ // The current array of items (some of which are null)
+ // All non-null objects are at the beginning of the array
+ // and the array is reorganized on "remove"
+ @SuppressWarnings({"unchecked"})
+ private T[] list = (T[])new Object[DEFAULT_SIZE];
+ // The current length of the active list
+ private int last = 0;
/**
* Adds a listener to the list, if it has not previously been added.
@@ -82,27 +74,17 @@ public abstract class ListenerList<T> im
* @param listener
*/
public void add(T listener) {
- if (listener == null) {
- throw new IllegalArgumentException("listener is null.");
+ if (indexOf(listener) >= 0) {
+ System.err.println("Duplicate listener " + listener + " added to " + this);
+ return;
}
- Node node = first;
-
- if (node == null) {
- first = new Node(null, null, listener);
- } else {
- while (node.next != null
- && node.listener != listener) {
- node = node.next;
- }
-
- if (node.next == null
- && node.listener != listener) {
- node.next = new Node(node, null, listener);
- } else {
- System.err.println("Duplicate listener " + listener + " added to " + this);
- }
+ // If no slot is available, increase the size of the array
+ if (last >= list.length) {
+ list = Arrays.copyOf(list, list.length + DEFAULT_SIZE);
}
+
+ list[last++] = listener;
}
/**
@@ -111,33 +93,32 @@ public abstract class ListenerList<T> im
* @param listener
*/
public void remove(T listener) {
- if (listener == null) {
- throw new IllegalArgumentException("listener is null.");
+ int slot = indexOf(listener);
+
+ if (slot < 0) {
+ System.err.println("Nonexistent listener " + listener + " removed from " + this);
+ return;
}
- Node node = first;
- while (node != null
- && node.listener != listener) {
- node = node.next;
+ // Once we find the entry in the list, copy the rest of the
+ // existing entries down by one position
+ if (slot < last - 1) {
+ System.arraycopy(list, slot + 1, list, slot, last - 1 - slot);
}
- if (node == null) {
- System.err.println("Nonexistent listener " + listener + " removed from " + this);
- } else {
- if (node.previous == null) {
- first = node.next;
-
- if (first != null) {
- first.previous = null;
- }
- } else {
- node.previous.next = node.next;
-
- if (node.next != null) {
- node.next.previous = node.previous;
- }
+ list[--last] = null;
+ }
+
+ private int indexOf(T listener) {
+ if (listener == null) {
+ throw new IllegalArgumentException("listener is null.");
+ }
+ for (int i = 0; i < last; i++) {
+ if (list[i] == listener) {
+ return i;
}
}
+ return -1;
}
/**
@@ -150,17 +131,7 @@ public abstract class ListenerList<T> im
* otherwise.
*/
public boolean contains(T listener) {
- if (listener == null) {
- throw new IllegalArgumentException("listener is null.");
- }
-
- Node node = first;
- while (node != null
- && node.listener != listener) {
- node = node.next;
- }
-
- return (node != null);
+ return indexOf(listener) >= 0;
}
/**
@@ -171,11 +142,12 @@ public abstract class ListenerList<T> im
* otherwise.
*/
public boolean isEmpty() {
- return (first == null);
+ return last == 0;
}
@Override
public Iterator<T> iterator() {
return new NodeIterator();
}
+
}
Modified: pivot/branches/2.0.x/wtk/src/org/apache/pivot/wtk/content/ButtonData.java
URL: http://svn.apache.org/viewvc/pivot/branches/2.0.x/wtk/src/org/apache/pivot/wtk/content/ButtonData.java?rev=1447977&r1=1447976&r2=1447977&view=diff
==============================================================================
--- pivot/branches/2.0.x/wtk/src/org/apache/pivot/wtk/content/ButtonData.java (original)
+++ pivot/branches/2.0.x/wtk/src/org/apache/pivot/wtk/content/ButtonData.java Wed Feb 20 00:13:32 2013
@@ -71,16 +71,15 @@ public class ButtonData {
}
Image iconLocal = (Image)ApplicationContext.getResourceCache().get(iconURL);
+
if (iconLocal == null) {
try {
iconLocal = Image.load(iconURL);
- ApplicationContext.getResourceCache().put(iconURL, iconLocal);
} catch (TaskExecutionException exception) {
throw new IllegalArgumentException(exception);
}
- } else {
- iconLocal.clearImageListeners();
+ ApplicationContext.getResourceCache().put(iconURL, iconLocal);
}
setIcon(iconLocal);
Modified: pivot/branches/2.0.x/wtk/src/org/apache/pivot/wtk/media/Image.java
URL: http://svn.apache.org/viewvc/pivot/branches/2.0.x/wtk/src/org/apache/pivot/wtk/media/Image.java?rev=1447977&r1=1447976&r2=1447977&view=diff
==============================================================================
--- pivot/branches/2.0.x/wtk/src/org/apache/pivot/wtk/media/Image.java (original)
+++ pivot/branches/2.0.x/wtk/src/org/apache/pivot/wtk/media/Image.java Wed Feb 20 00:13:32 2013
@@ -139,10 +139,6 @@ public abstract class Image implements V
return imageListeners;
}
- public void clearImageListeners() {
- this.imageListeners = new ImageListenerList();
- }
-
public static Image load(URL location) throws TaskExecutionException {
LoadTask loadTask = new LoadTask(location);
return loadTask.execute();