rdonkin 2004/11/17 15:23:22
Modified: logging/optional/src/java/org/apache/commons/logging/impl
WeakHashtable.java
logging/optional/src/test/org/apache/commons/logging
LogFactoryTest.java
logging/optional/src/test/org/apache/commons/logging/impl
WeakHashtableTest.java
Log:
Improvements to WeakHashTable. Values are now held with hard references and a
reference queue is polled during a purge. Contributed by Brian Stansberry.
Revision Changes Path
1.4 +120 -27
jakarta-commons/logging/optional/src/java/org/apache/commons/logging/impl/WeakHashtable.java
Index: WeakHashtable.java
===================================================================
RCS file:
/home/cvs/jakarta-commons/logging/optional/src/java/org/apache/commons/logging/impl/WeakHashtable.java,v
retrieving revision 1.3
retrieving revision 1.4
diff -u -r1.3 -r1.4
--- WeakHashtable.java 11 Nov 2004 22:31:05 -0000 1.3
+++ WeakHashtable.java 17 Nov 2004 23:23:21 -0000 1.4
@@ -17,6 +17,7 @@
package org.apache.commons.logging.impl;
+import java.lang.ref.ReferenceQueue;
import java.lang.ref.WeakReference;
import java.util.*;
@@ -37,12 +38,28 @@
* running 1.3+ JVMs. Use of this class will allow classloaders to be
collected by
* the garbage collector without the need to call release.
* </p>
+ *
+ * @author Brian Stansberry
*/
public final class WeakHashtable extends Hashtable {
/** Empty array of <code>Entry</code>'s */
private static final Entry[] EMPTY_ENTRY_ARRAY = {};
+ /**
+ * The maximum number of times put() can be called before
+ * the map will purged of cleared entries.
+ */
+ public static final int MAX_PUTS_BEFORE_PURGE = 100;
+
+ /* ReferenceQueue we check for gc'd keys */
+ private ReferenceQueue queue = new ReferenceQueue();
+ /* Counter used to control how often we purge gc'd entries */
+ private int putCount = 0;
+ /**
+ * Constructs a WeakHashtable with the Hashtable default
+ * capacity and load factor.
+ */
public WeakHashtable() {}
/**
@@ -169,7 +186,6 @@
[EMAIL PROTECTED] Hashtable
*/
public Object put(Object key, Object value) {
- // for performance reasons, no purge
// check for nulls, ensuring symantics match superclass
if (key == null) {
throw new NullPointerException("Null keys are not allowed");
@@ -177,9 +193,17 @@
if (value == null) {
throw new NullPointerException("Null values are not allowed");
}
-
+
+ // for performance reasons, only purge every
+ // MAX_PUTS_BEFORE_PURGE times
+ if (putCount++ > MAX_PUTS_BEFORE_PURGE) {
+ purge();
+ putCount = 0;
+ }
Object result = null;
- Referenced lastValue = (Referenced) super.put(new Referenced(key),
new Referenced(value));
+ Referenced keyRef = new Referenced(key, value, queue);
+ Referenced valueRef = new Referenced(value);
+ Referenced lastValue = (Referenced) super.put(keyRef, valueRef);
if (lastValue != null) {
result = lastValue.getValue();
}
@@ -248,31 +272,30 @@
}
/**
- * Purges all entries whose wrapped keys or values
+ * @see Hashtable
+ */
+ protected void rehash() {
+ // purge here to save the effort of rehashing dead entries
+ purge();
+ super.rehash();
+ }
+
+ /**
+ * Purges all entries whose wrapped keys
* have been garbage collected.
*/
private synchronized void purge() {
- Set entrySet = super.entrySet();
- for (Iterator it=entrySet.iterator(); it.hasNext();) {
- Map.Entry entry = (Map.Entry) it.next();
- Referenced referencedKey = (Referenced) entry.getKey();
- Referenced referencedValue = (Referenced) entry.getValue();
-
- // test whether either referant has been collected
- if (referencedKey.getValue() == null ||
referencedValue.getValue() == null) {
- // if so, purge this entry
- it.remove();
- }
+ WeakKey key;
+ while ( (key = (WeakKey) queue.poll()) != null) {
+ super.remove(key.getReferenced());
}
}
-
-
/** Entry implementation */
private final static class Entry implements Map.Entry {
- private Object key;
- private Object value;
+ private final Object key;
+ private final Object value;
private Entry(Object key, Object value) {
this.key = key;
@@ -318,18 +341,33 @@
private final static class Referenced {
private final WeakReference reference;
-
+ private final int hashCode;
+
+ /**
+ *
+ * @throws NullPointerException if referant is <code>null</code>
+ */
private Referenced(Object referant) {
reference = new WeakReference(referant);
+ // Calc a permanent hashCode so calls to Hashtable.remove()
+ // work if the WeakReference has been cleared
+ hashCode = referant.hashCode();
+ }
+
+ /**
+ *
+ * @throws NullPointerException if key is <code>null</code>
+ */
+ private Referenced(Object key, Object value, ReferenceQueue queue) {
+ reference = new WeakKey(key, value, queue, this);
+ // Calc a permanent hashCode so calls to Hashtable.remove()
+ // work if the WeakReference has been cleared
+ hashCode = key.hashCode();
+
}
public int hashCode() {
- int result = 0;
- Object keyValue = getValue();
- if (keyValue != null) {
- result = keyValue.hashCode();
- }
- return result;
+ return hashCode;
}
private Object getValue() {
@@ -342,8 +380,22 @@
Referenced otherKey = (Referenced) o;
Object thisKeyValue = getValue();
Object otherKeyValue = otherKey.getValue();
- if (thisKeyValue == null) {
+ if (thisKeyValue == null) {
result = (otherKeyValue == null);
+
+ // Since our hashcode was calculated from the original
+ // non-null referant, the above check breaks the
+ // hashcode/equals contract, as two cleared Referenced
+ // objects could test equal but have different hashcodes.
+ // We can reduce (not eliminate) the chance of this
+ // happening by comparing hashcodes.
+ if (result == true) {
+ result = (this.hashCode() == otherKey.hashCode());
+ }
+ // In any case, as our c'tor does not allow null
referants
+ // and Hashtable does not do equality checks between
+ // existing keys, normal hashtable operations should
never
+ // result in an equals comparison between null referants
}
else
{
@@ -353,4 +405,45 @@
return result;
}
}
+
+ /**
+ * WeakReference subclass that holds a hard reference to an
+ * associated <code>value</code> and also makes accessible
+ * the Referenced object holding it.
+ */
+ private final static class WeakKey extends WeakReference {
+
+ private final Object hardValue;
+ private final Referenced referenced;
+
+ private WeakKey(Object key,
+ Object value,
+ ReferenceQueue queue,
+ Referenced referenced) {
+ super(key, queue);
+ hardValue = value;
+ this.referenced = referenced;
+ }
+
+ private Referenced getReferenced() {
+ return referenced;
+ }
+
+ /* Drop our hard reference to value if we've been cleared
+ * by the gc.
+ *
+ * Testing shows that with key objects like ClassLoader
+ * that don't override hashCode(), get() is never
+ * called once the key is in a Hashtable.
+ * So, this method override is commented out.
+ */
+ //public Object get() {
+ // Object result = super.get();
+ // if (result == null) {
+ // // We've been cleared, so drop our hard reference to value
+ // hardValue = null;
+ // }
+ // return result;
+ //}
+ }
}
1.2 +174 -2
jakarta-commons/logging/optional/src/test/org/apache/commons/logging/LogFactoryTest.java
Index: LogFactoryTest.java
===================================================================
RCS file:
/home/cvs/jakarta-commons/logging/optional/src/test/org/apache/commons/logging/LogFactoryTest.java,v
retrieving revision 1.1
retrieving revision 1.2
diff -u -r1.1 -r1.2
--- LogFactoryTest.java 10 Nov 2004 22:59:39 -0000 1.1
+++ LogFactoryTest.java 17 Nov 2004 23:23:21 -0000 1.2
@@ -17,8 +17,12 @@
package org.apache.commons.logging;
-import junit.framework.*;
-import java.util.*;
+import java.lang.ref.WeakReference;
+import java.util.Hashtable;
+
+import junit.framework.TestCase;
+
+import org.apache.commons.logging.impl.LogFactoryImpl;
import org.apache.commons.logging.impl.WeakHashtable;
public class LogFactoryTest extends TestCase {
@@ -27,6 +31,8 @@
/** Maximum number of iterations before our test fails */
private static final int MAX_GC_ITERATIONS = 50;
+ private ClassLoader origLoader = null;
+ private String origFactoryProperty = null;
public LogFactoryTest(String testName) {
super(testName);
@@ -35,4 +41,170 @@
public void testLogFactoryType() {
assertTrue(LogFactory.factories instanceof WeakHashtable);
}
+
+ /**
+ * Tests that LogFactories are not removed from the map
+ * if their creating ClassLoader is still alive.
+ */
+ public void testHoldFactories()
+ {
+ // Get a factory and create a WeakReference to it that
+ // we can check to see if the factory has been removed
+ // from LogFactory.properties
+ LogFactory factory = LogFactory.getFactory();
+ WeakReference weakFactory = new WeakReference(factory);
+
+ // Remove any hard reference to the factory
+ factory = null;
+
+ // Run the gc, confirming that the original factory
+ // is not dropped from the map even though there are
+ // no other references to it
+ int iterations = 0;
+ int bytz = 2;
+ while(iterations++ < MAX_GC_ITERATIONS) {
+ System.gc();
+
+ assertNotNull("LogFactory released while ClassLoader still
active.",
+ weakFactory.get());
+
+ // create garbage:
+ byte[] b;
+ try {
+ b = new byte[bytz];
+ bytz = bytz * 2;
+ }
+ catch (OutOfMemoryError oom) {
+ // This error means the test passed, as it means the
LogFactory
+ // was never released. So, we have to catch and deal with it
+
+ // Doing this is probably a no-no, but it seems to work ;-)
+ b = null;
+ System.gc();
+ break;
+ }
+ }
+ }
+
+ /**
+ * Tests that a LogFactory is eventually removed from the map
+ * after its creating ClassLoader is garbage collected.
+ */
+ public void testReleaseFactories()
+ {
+ // Create a temporary classloader
+ ClassLoader childLoader = new ClassLoader() {};
+ Thread.currentThread().setContextClassLoader(childLoader);
+
+ // Get a factory using the child loader.
+ LogFactory factory = LogFactory.getFactory();
+ // Hold a WeakReference to the factory. When this reference
+ // is cleared we know the factory has been cleared from
+ // LogFactory.factories as well
+ WeakReference weakFactory = new WeakReference(factory);
+
+ // Get a WeakReference to the child loader so we know when it
+ // has been gc'ed
+ WeakReference weakLoader = new WeakReference(childLoader);
+
+ // Remove any hard reference to the childLoader and the factory
+ Thread.currentThread().setContextClassLoader(origLoader);
+ childLoader = null;
+ factory = null;
+
+ // Run the gc, confirming that the original childLoader
+ // is dropped from the map
+ int iterations = 0;
+ int bytz = 2;
+ while(true) {
+ System.gc();
+ if(iterations++ > MAX_GC_ITERATIONS){
+ fail("Max iterations reached before childLoader released.");
+ }
+
+ if(weakLoader.get() == null) {
+ break;
+ } else {
+ // create garbage:
+ byte[] b;
+ try {
+ b = new byte[bytz];
+ bytz = bytz * 2;
+ }
+ catch (OutOfMemoryError oom) {
+ // Doing this is probably a no-no, but it seems to work
;-)
+ b = null;
+ System.gc();
+ fail("OutOfMemory before childLoader released.");
+ }
+ }
+ }
+
+ // Confirm that the original factory is removed from the map
+ // within the maximum allowed number of calls to put() +
+ // the maximum number of subsequent gc iterations
+ iterations = 0;
+ while(true) {
+ System.gc();
+ if(iterations++ > WeakHashtable.MAX_PUTS_BEFORE_PURGE +
MAX_GC_ITERATIONS){
+ Hashtable table = LogFactory.factories;
+ fail("Max iterations reached before factory released.");
+ }
+
+ // Create a new child loader and use it to add to the map.
+ ClassLoader newChildLoader = new ClassLoader() {};
+ Thread.currentThread().setContextClassLoader(newChildLoader);
+ LogFactory.getFactory();
+ Thread.currentThread().setContextClassLoader(origLoader);
+
+ if(weakFactory.get() == null) {
+ break;
+ } else {
+ // create garbage:
+ byte[] b;
+ try {
+ b = new byte[bytz];
+ bytz = bytz * 2;
+ }
+ catch (OutOfMemoryError oom) {
+ // Doing this is probably a no-no, but it seems to work
;-)
+ b = null;
+ bytz = 2; // start over
+ System.gc();
+ }
+ }
+ }
+
+ }
+
+ protected void setUp() throws Exception {
+ // Preserve the original classloader and factory implementation
+ // class so we can restore them when we are done
+ origLoader = Thread.currentThread().getContextClassLoader();
+ origFactoryProperty =
System.getProperty(LogFactory.FACTORY_PROPERTY);
+
+ // Ensure we use LogFactoryImpl as our factory
+ System.setProperty(LogFactory.FACTORY_PROPERTY,
+ LogFactoryImpl.class.getName());
+
+ super.setUp();
+ }
+
+ protected void tearDown() throws Exception {
+ // Set the classloader back to whatever it originally was
+ Thread.currentThread().setContextClassLoader(origLoader);
+
+ // Set the factory implementation class back to
+ // whatever it originally was
+ if (origFactoryProperty != null) {
+ System.setProperty(LogFactory.FACTORY_PROPERTY,
+ origFactoryProperty);
+ }
+ else {
+ System.getProperties().remove(LogFactory.FACTORY_PROPERTY);
+ }
+
+ super.tearDown();
+ }
+
}
1.4 +11 -1
jakarta-commons/logging/optional/src/test/org/apache/commons/logging/impl/WeakHashtableTest.java
Index: WeakHashtableTest.java
===================================================================
RCS file:
/home/cvs/jakarta-commons/logging/optional/src/test/org/apache/commons/logging/impl/WeakHashtableTest.java,v
retrieving revision 1.3
retrieving revision 1.4
diff -u -r1.3 -r1.4
--- WeakHashtableTest.java 11 Nov 2004 22:31:05 -0000 1.3
+++ WeakHashtableTest.java 17 Nov 2004 23:23:22 -0000 1.4
@@ -17,6 +17,7 @@
package org.apache.commons.logging.impl;
+import java.lang.ref.*;
import junit.framework.*;
import java.util.*;
@@ -206,7 +207,10 @@
}
public void testRelease() throws Exception {
-
+ assertNotNull(weakHashtable.get(new Long(1)));
+ ReferenceQueue testQueue = new ReferenceQueue();
+ WeakReference weakKeyOne = new WeakReference(keyOne, testQueue);
+
// lose our references
keyOne = null;
keyTwo = null;
@@ -232,6 +236,12 @@
bytz = bytz * 2;
}
}
+
+ // some JVMs seem to take a little time to put references on
+ // the reference queue once the reference has been collected
+ // need to think about whether this is enough to justify
+ // stepping through the collection each time...
+ while(testQueue.poll() == null) {}
// Test that the released objects are not taking space in the table
assertEquals("underlying table not emptied", 0,
weakHashtable.size());
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
