Re: [collections] ReferenceMap changed for WeakIdentityMap but now broken
Thanks Phil, that was the problem. ReferenceMap now extends
AbstractHashedMap ;-)
Stephen
- Original Message -
From: Phil Steitz [EMAIL PROTECTED]
I think the problem may be in the first line below. I don't think you
want to hash the reference. If you change hash(ref) to ref.hashCode() all
tests (including commented out ones) succeed.
private void purge(Reference ref) {
// The hashCode of the reference is the hashCode of the
// mapping key, even if the reference refers to the
// mapping value...
int hash = hash(ref);
int index = hashIndex(hash, data.length);
HashEntry previous = null;
HashEntry entry = data[index];
while (entry != null) {
if (((ReferenceEntry) entry).purge(ref)) {
if (previous == null) {
data[index] = entry.next;
} else {
previous.next = entry.next;
}
this.size--;
return;
}
previous = entry;
entry = entry.next;
}
}
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
Re: [collections] ReferenceMap changed for WeakIdentityMap but now broken
I think the problem may be in the first line below. I don't think you
want to hash the reference. If you change hash(ref) to ref.hashCode() all
tests (including commented out ones) succeed.
private void purge(Reference ref) {
// The hashCode of the reference is the hashCode of the
// mapping key, even if the reference refers to the
// mapping value...
int hash = hash(ref);
int index = hashIndex(hash, data.length);
HashEntry previous = null;
HashEntry entry = data[index];
while (entry != null) {
if (((ReferenceEntry) entry).purge(ref)) {
if (previous == null) {
data[index] = entry.next;
} else {
previous.next = entry.next;
}
this.size--;
return;
}
previous = entry;
entry = entry.next;
}
}
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
Re: [collections] ReferenceMap changed for WeakIdentityMap but now broken
Janek Bogucki wrote: On Wed, 2004-03-24 at 21:57, Janek Bogucki wrote: At the end there is this which you did not mention in your original post so it might be new information [junit] Testcase: testPurgeValues took 4.903 sec [junit] Caused an ERROR [junit] null [junit] java.lang.OutOfMemoryError [junit] no stack trace available [junit] Testcase: testPurgeValues I now realize that this is actually how the test is intended to work. Sorry for the noise. It looks to me as though the intention would be to fail with Max iterations reached before resource released which happens if you decrease the bound on the loop to, say 20. This should probably be changed. The null test that is failing is valueReference.get() == null I have not fully grokked the code, so the following is speculation, but might be helpful. The test setup is WeakReference keyReference = new WeakReference(key); WeakReference valueReference = new WeakReference(value); Map testMap = new ReferenceMap(ReferenceMap.WEAK, ReferenceMap.HARD, true); so the valueReference release has to be triggered by releasing the key reference, but in ReferenceEntry.purge(ref), there is no check for purgeValues. Phil -Janek - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [collections] ReferenceMap changed for WeakIdentityMap but now broken
On Tue, 2004-03-23 at 23:01, Stephen Colebourne wrote:
I can't spot the problem, so I'm posting it in case someone else wants to
take a look and tell me my stupid mistake ;-)
Stephen
The error message from the batch test was unhelpful. This is what I get
with this task added before the batch test
junit haltonerror=true haltonfailure=true printsummary=no
classpath
pathelement location=${build.classes}/
pathelement location=${build.tests}/
pathelement location=${junit.jar}/
/classpath
formatter type=plain usefile=false/
test name=org.apache.commons.collections.map.TestReferenceMap/
/junit
At the end there is this which you did not mention in your original post
so it might be new information
[junit] Testcase: testPurgeValues took 4.903 sec
[junit] Caused an ERROR
[junit] null
[junit] java.lang.OutOfMemoryError
[junit] no stack trace available
[junit] Testcase: testPurgeValues
Full dump
-
test:
[junit] Testsuite:
org.apache.commons.collections.map.TestReferenceMap
[junit] Tests run: 115, Failures: 0, Errors: 1, Time elapsed: 5.373
sec
[junit] Testcase: testObjectEqualsSelf took 0.027 sec
[junit] Testcase: testEqualsNull took 0.001 sec
[junit] Testcase: testObjectHashCodeEqualsSelfHashCode took 0.003
sec
[junit] Testcase: testObjectHashCodeEqualsContract took 0.02 sec
[junit] Testcase: testSerializeDeserializeThenCompare took 0 sec
[junit] Testcase: testSimpleSerialization took 0 sec
[junit] Testcase: testCanonicalEmptyCollectionExists took 0.001 sec
[junit] Testcase: testCanonicalFullCollectionExists took 0 sec
[junit] Testcase: testSampleMappings took 0.001 sec
[junit] Testcase: testMakeMap took 0.004 sec
[junit] Testcase: testMapIsEmpty took 0.014 sec
[junit] Testcase: testMapSize took 0.006 sec
[junit] Testcase: testMapClear took 0.001 sec
[junit] Testcase: testMapContainsKey took 0.01 sec
[junit] Testcase: testMapContainsValue took 0.006 sec
[junit] Testcase: testMapEquals took 0.009 sec
[junit] Testcase: testMapGet took 0 sec
[junit] Testcase: testMapHashCode took 0.003 sec
[junit] Testcase: testMapToString took 0.002 sec
[junit] Testcase: testEmptyMapCompatibility took 0.001 sec
[junit] Testcase: testFullMapCompatibility took 0 sec
[junit] Testcase: testMapPut took 0.05 sec
[junit] Testcase: testMapPutAll took 0.004 sec
[junit] Testcase: testMapRemove took 0.021 sec
[junit] Testcase: testValuesClearChangesMap took 0 sec
[junit] Testcase: testKeySetClearChangesMap took 0.002 sec
[junit] Testcase: testEntrySetClearChangesMap took 0.001 sec
[junit] Testcase: testValuesRemoveChangesMap took 0.001 sec
[junit] Testcase: testKeySetRemoveChangesMap took 0 sec
[junit] Testcase: testObjectEqualsSelf took 0.001 sec
[junit] Testcase: testEqualsNull took 0 sec
[junit] Testcase: testObjectHashCodeEqualsSelfHashCode took 0 sec
[junit] Testcase: testObjectHashCodeEqualsContract took 0 sec
[junit] Testcase: testSerializeDeserializeThenCompare took 0 sec
[junit] Testcase: testSimpleSerialization took 0 sec
[junit] Testcase: testCanonicalEmptyCollectionExists took 0 sec
[junit] Testcase: testCanonicalFullCollectionExists took 0.001 sec
[junit] Testcase: testCollectionAdd took 0 sec
[junit] Testcase: testCollectionAddAll took 0 sec
[junit] Testcase: testUnsupportedAdd took 0.009 sec
[junit] Testcase: testCollectionClear took 0.001 sec
[junit] Testcase: testCollectionContains took 0.002 sec
[junit] Testcase: testCollectionContainsAll took 0.009 sec
[junit] Testcase: testCollectionIsEmpty took 0.003 sec
[junit] Testcase: testCollectionIterator took 0.006 sec
[junit] Testcase: testCollectionIteratorRemove took 0.019 sec
[junit] Testcase: testCollectionRemove took 0.061 sec
[junit] Testcase: testCollectionRemoveAll took 0.007 sec
[junit] Testcase: testCollectionRetainAll took 0.006 sec
[junit] Testcase: testCollectionSize took 0.002 sec
[junit] Testcase: testCollectionToArray took 0.006 sec
[junit] Testcase: testCollectionToArray2 took 0.006 sec
[junit] Testcase: testCollectionToString took 0 sec
[junit] Testcase: testUnsupportedRemove took 0 sec
[junit] Testcase: testCollectionIteratorFailFast took 0 sec
[junit] Testcase: testSetEquals took 0.004 sec
[junit] Testcase: testSetHashCode took 0 sec
[junit] Testcase: testMapEntrySetIteratorEntry took 0.001 sec
[junit] Testcase: testMapEntrySetIteratorEntrySetValue took 0.003
sec
[junit] Testcase: testMapEntrySetRemoveNonMapEntry took 0.001 sec
[junit] Testcase: testObjectEqualsSelf took 0 sec
[junit] Testcase: testEqualsNull took 0 sec
[junit] Testcase: testObjectHashCodeEqualsSelfHashCode took 0 sec
[junit] Testcase: testObjectHashCodeEqualsContract took 0.001 sec
[junit] Testcase:
Re: [collections] ReferenceMap changed for WeakIdentityMap but now broken
On Wed, 2004-03-24 at 21:57, Janek Bogucki wrote: At the end there is this which you did not mention in your original post so it might be new information [junit] Testcase: testPurgeValues took 4.903 sec [junit] Caused an ERROR [junit] null [junit] java.lang.OutOfMemoryError [junit] no stack trace available [junit] Testcase: testPurgeValues I now realize that this is actually how the test is intended to work. Sorry for the noise. -Janek - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [collections] ReferenceMap changed for WeakIdentityMap but now broken
On Tue, 2004-03-23 at 23:01, Stephen Colebourne wrote:
I can't spot the problem, so I'm posting it in case someone else wants to
take a look and tell me my stupid mistake ;-)
Sorry this isn't expressed as unit test but just in case this is enough
to go on I've posted it. I added a System.out.println to
testPurgeValues() just before the invocation of isEmpty()
System.out.println(testPurgeValues: before isEmpty: + iterations );
testMap.isEmpty();
then two more in purge(Reference) to get this output
[junit] testPurgeValues: before isEmpty: 1
[junit] purge()
[junit] purge(Reference)
[junit] purge(Reference): entry: null
[junit] testPurgeValues: before isEmpty: 2
[junit] purge()
[junit] testPurgeValues: before isEmpty: 3
[junit] purge()
[junit] testPurgeValues: before isEmpty: 4
[junit] purge()
[junit] testPurgeValues: before isEmpty: 5
So purge correctly invokes purge(Reference) but purge(Reference) gets
null for entry and never enters the while block.
private void purge(Reference ref) {
System.out.println(purge(Reference));
// The hashCode of the reference is the hashCode of the
// mapping key, even if the reference refers to the
// mapping value...
int hash = hash(ref);
int index = hashIndex(hash, data.length);
HashEntry previous = null;
HashEntry entry = data[index];
System.out.println(purge(Reference): entry: + entry);
while (entry != null) {
if (((ReferenceEntry) entry).purge(ref)) {
if (previous == null) {
data[index] = entry.next;
} else {
previous.next = entry.next;
}
this.size--;
return;
}
previous = entry;
entry = entry.next;
}
}
-Janek
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
[collections] ReferenceMap changed for WeakIdentityMap but now broken
I altered ReferenceMap to extend AbstractHashedMap, which seems to be
possible to do in a backwards compatable way (prior to looking at crreating
a WeakIdentityMap as per recent requests). The new version is attached.
However, the new version doesn't seem to work, as in the 'reference'
(weak/soft) aspect seems to have got broken in my changes. (The test cases
suceed on the old code, but fail on the new code, including the commented
out test cases)
I can't spot the problem, so I'm posting it in case someone else wants to
take a look and tell me my stupid mistake ;-)
Stephen
/*
* Copyright 2002-2004 The Apache Software Foundation
*
* Licensed under the Apache License, Version 2.0 (the License);
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an AS IS BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.apache.commons.collections.map;
import java.io.IOException;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
import java.lang.ref.Reference;
import java.lang.ref.ReferenceQueue;
import java.lang.ref.SoftReference;
import java.lang.ref.WeakReference;
import java.util.ArrayList;
import java.util.Collection;
import java.util.ConcurrentModificationException;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.NoSuchElementException;
import java.util.Set;
import org.apache.commons.collections.keyvalue.DefaultMapEntry;
/**
* Hash-based codeMap/code implementation that allows
* mappings to be removed by the garbage collector.
* p
* When you construct a codeReferenceMap/code, you can
* specify what kind of references are used to store the
* map's keys and values. If non-hard references are
* used, then the garbage collector can remove mappings
* if a key or value becomes unreachable, or if the
* JVM's memory is running low. For information on how
* the different reference types behave, see
* [EMAIL PROTECTED] Reference}.
* p
* Different types of references can be specified for keys
* and values. The keys can be configured to be weak but
* the values hard, in which case this class will behave
* like a a href=http://java.sun.com/j2se/1.4/docs/api/java/util/WeakHashMap.html;
* codeWeakHashMap/code/a. However, you
* can also specify hard keys and weak values, or any other
* combination. The default constructor uses hard keys
* and soft values, providing a memory-sensitive cache.
* p
* The algorithms used are basically the same as those
* in [EMAIL PROTECTED] java.util.HashMap}. In particular, you
* can specify a load factor and capacity to suit your
* needs. All optional [EMAIL PROTECTED] Map} operations are
* supported.
* p
* However, this [EMAIL PROTECTED] Map} implementation does Inot/I
* allow null elements. Attempting to add a null key or
* or a null value to the map will raise a
* codeNullPointerException/code.
* p
* As usual, this implementation is not synchronized. You
* can use [EMAIL PROTECTED] java.util.Collections#synchronizedMap} to
* provide synchronized access to a codeReferenceMap/code.
*
* @see java.lang.ref.Reference
*
* @since Commons Collections 3.0 (previously in main package v2.1)
* @version $Revision: 1.10 $ $Date: 2004/02/18 01:13:19 $
*
* @author Paul Jack
*/
public class ReferenceMap extends AbstractHashedMap {
/**
* For serialization.
*/
private static final long serialVersionUID = -3370601314380922368L;
/**
* Constant indicating that hard references should be used.
*/
public static final int HARD = 0;
/**
* Constant indicating that soft references should be used.
*/
public static final int SOFT = 1;
/**
* Constant indicating that weak references should be used.
*/
public static final int WEAK = 2;
// --- serialized instance variables:
/**
* The reference type for keys. Must be HARD, SOFT, WEAK.
* Note: I originally marked this field as final, but then this class
* didn't compile under JDK1.2.2.
* @serial
*/
private int keyType;
/**
* The reference type for values. Must be HARD, SOFT, WEAK.
* Note: I originally marked this field as final, but then this class
* didn't compile under JDK1.2.2.
* @serial
*/
private int valueType;
/**
* The threshold variable is calculated by multiplying
* table.length and loadFactor.
* Note: I originally marked this field as final, but then this class
* didn't compile under JDK1.2.2.
* @serial
*/
private float loadFactor;
/**
RE: [collections] ReferenceMap changed for WeakIdentityMap but now broken
Stephen,
I only spent a minute or two looking, but:
public Object put(Object key, Object value) {
if (key == null) {
throw new NullPointerException(null keys not allowed);
}
if (value == null) {
throw new NullPointerException(null values not allowed);
}
purge();
return super.put(key, value);
}
Where exactly does the Reference aspect come into that? Compare that to the
original code.
--- Noel
-Original Message-
From: Stephen Colebourne [mailto:[EMAIL PROTECTED]
Sent: Tuesday, March 23, 2004 18:02
To: Jakarta Commons Developers List
Subject: [collections] ReferenceMap changed for WeakIdentityMap but now
broken
I altered ReferenceMap to extend AbstractHashedMap, which seems to be
possible to do in a backwards compatable way (prior to looking at crreating
a WeakIdentityMap as per recent requests). The new version is attached.
However, the new version doesn't seem to work, as in the 'reference'
(weak/soft) aspect seems to have got broken in my changes. (The test cases
suceed on the old code, but fail on the new code, including the commented
out test cases)
I can't spot the problem, so I'm posting it in case someone else wants to
take a look and tell me my stupid mistake ;-)
Stephen
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
Re: [collections] ReferenceMap changed for WeakIdentityMap but now broken
The put(key, value) calls the superclass, which then calls createEntry() to
create an entry. The entry should be of the subclass ReferenceEntry which
should handle the references in the constructor.
Stephen
- Original Message -
From: Noel J. Bergman [EMAIL PROTECTED]
I only spent a minute or two looking, but:
public Object put(Object key, Object value) {
if (key == null) {
throw new NullPointerException(null keys not allowed);
}
if (value == null) {
throw new NullPointerException(null values not allowed);
}
purge();
return super.put(key, value);
}
Where exactly does the Reference aspect come into that? Compare that to
the
original code.
--- Noel
-Original Message-
From: Stephen Colebourne [mailto:[EMAIL PROTECTED]
Sent: Tuesday, March 23, 2004 18:02
To: Jakarta Commons Developers List
Subject: [collections] ReferenceMap changed for WeakIdentityMap but now
broken
I altered ReferenceMap to extend AbstractHashedMap, which seems to be
possible to do in a backwards compatable way (prior to looking at
crreating
a WeakIdentityMap as per recent requests). The new version is attached.
However, the new version doesn't seem to work, as in the 'reference'
(weak/soft) aspect seems to have got broken in my changes. (The test cases
suceed on the old code, but fail on the new code, including the commented
out test cases)
I can't spot the problem, so I'm posting it in case someone else wants to
take a look and tell me my stupid mistake ;-)
Stephen
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
