You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Stephen Colebourne <sc...@btopenworld.com> on 2004/03/24 00:01:38 UTC
[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
Re: [collections] ReferenceMap changed for WeakIdentityMap but now broken
Posted by Stephen Colebourne <sc...@btopenworld.com>.
Thanks Phil, that was the problem. ReferenceMap now extends
AbstractHashedMap ;-)
Stephen
----- Original Message -----
From: "Phil Steitz" <ph...@steitz.com>
> 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: commons-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-dev-help@jakarta.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [collections] ReferenceMap changed for WeakIdentityMap but now
broken
Posted by Phil Steitz <ph...@steitz.com>.
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: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [collections] ReferenceMap changed for WeakIdentityMap but now
broken
Posted by Phil Steitz <ph...@steitz.com>.
Phil Steitz wrote:
> 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.
Strike the last stmt above. There is a test and the reference is nulled.
Sorry.
-Phil
>
>
>
>>
>> -Janek
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
>> For additional commands, e-mail: commons-dev-help@jakarta.apache.org
>>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-dev-help@jakarta.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [collections] ReferenceMap changed for WeakIdentityMap but now
broken
Posted by Phil Steitz <ph...@steitz.com>.
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: commons-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-dev-help@jakarta.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [collections] ReferenceMap changed for WeakIdentityMap but now
broken
Posted by Janek Bogucki <ya...@studylink.com>.
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: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [collections] ReferenceMap changed for WeakIdentityMap but now
broken
Posted by Janek Bogucki <ya...@studylink.com>.
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: testSerializeDeserializeThenCompare took 0 sec
[junit] Testcase: testSimpleSerialization took 0 sec
[junit] Testcase: testCanonicalEmptyCollectionExists took 0 sec
[junit] Testcase: testCanonicalFullCollectionExists took 0 sec
[junit] Testcase: testCollectionAdd took 0 sec
[junit] Testcase: testCollectionAddAll took 0 sec
[junit] Testcase: testUnsupportedAdd took 0.005 sec
[junit] Testcase: testCollectionClear took 0 sec
[junit] Testcase: testCollectionContains took 0.004 sec
[junit] Testcase: testCollectionContainsAll took 0.003 sec
[junit] Testcase: testCollectionIsEmpty took 0.001 sec
[junit] Testcase: testCollectionIterator took 0.002 sec
[junit] Testcase: testCollectionIteratorRemove took 0.014 sec
[junit] Testcase: testCollectionRemove took 0.04 sec
[junit] Testcase: testCollectionRemoveAll took 0.005 sec
[junit] Testcase: testCollectionRetainAll took 0.005 sec
[junit] Testcase: testCollectionSize took 0 sec
[junit] Testcase: testCollectionToArray took 0 sec
[junit] Testcase: testCollectionToArray2 took 0.003 sec
[junit] Testcase: testCollectionToString took 0 sec
[junit] Testcase: testUnsupportedRemove took 0 sec
[junit] Testcase: testCollectionIteratorFailFast took 0 sec
[junit] Testcase: testSetEquals took 0.003 sec
[junit] Testcase: testSetHashCode took 0 sec
[junit] Testcase: testObjectEqualsSelf took 0 sec
[junit] Testcase: testEqualsNull took 0 sec
[junit] Testcase: testObjectHashCodeEqualsSelfHashCode took 0.001
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 sec
[junit] Testcase: testCollectionAdd took 0 sec
[junit] Testcase: testCollectionAddAll took 0 sec
[junit] Testcase: testUnsupportedAdd took 0.003 sec
[junit] Testcase: testCollectionClear took 0 sec
[junit] Testcase: testCollectionContains took 0.003 sec
[junit] Testcase: testCollectionContainsAll took 0.002 sec
[junit] Testcase: testCollectionIsEmpty took 0.003 sec
[junit] Testcase: testCollectionIterator took 0.001 sec
[junit] Testcase: testCollectionIteratorRemove took 0 sec
[junit] Testcase: testCollectionRemove took 0.017 sec
[junit] Testcase: testCollectionRemoveAll took 0.002 sec
[junit] Testcase: testCollectionRetainAll took 0.004 sec
[junit] Testcase: testCollectionSize took 0 sec
[junit] Testcase: testCollectionToArray took 0 sec
[junit] Testcase: testCollectionToArray2 took 0.003 sec
[junit] Testcase: testCollectionToString took 0 sec
[junit] Testcase: testUnsupportedRemove took 0 sec
[junit] Testcase: testCollectionIteratorFailFast took 0 sec
[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
BUILD FAILED
file:/home/yan/cvs.apache.org/jakarta-commons/collections/build.xml:175:
Test org.apache.commons.collections.map.TestReferenceMap failed
Total time: 8 seconds
-Janek
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [collections] ReferenceMap changed for WeakIdentityMap but now
broken
Posted by Janek Bogucki <ya...@studylink.com>.
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: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [collections] ReferenceMap changed for WeakIdentityMap but now broken
Posted by Stephen Colebourne <sc...@btopenworld.com>.
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" <no...@devtech.com>
> 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:scolebourne@btopenworld.com]
> 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: commons-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-dev-help@jakarta.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
RE: [collections] ReferenceMap changed for WeakIdentityMap but now broken
Posted by "Noel J. Bergman" <no...@devtech.com>.
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:scolebourne@btopenworld.com]
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: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org