You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by robert burrell donkin <ro...@blueyonder.co.uk> on 2005/11/05 12:16:15 UTC

[collections] any objections?

i'd like to dive in with some improvements to the javadocs. i'm going
add myself to the list of committers and get started sometime soonish. 

so now would be a good time for people to jump in with any objections,
comments or warnings...

- robert


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [collections] any objections?

Posted by robert burrell donkin <ro...@blueyonder.co.uk>.
On Tue, 2005-11-15 at 13:07 -0500, Michael Heuer wrote:
> On Sat, 12 Nov 2005, Phil Steitz wrote:
> 
> > On 11/12/05, robert burrell donkin <ro...@blueyonder.co.uk> wrote:
> > > On Sat, 2005-11-12 at 13:44 +0000, Stephen Colebourne wrote:
> > > > robert burrell donkin wrote:
> > > > > has anyone run a long stress test?
> > > > >
> > > > > if not, i'm willing to code up something and set it running on my debian
> > > > > box for a few days. i'd appreciate a second pair of eyes on the code (to
> > > > > avoid mistakes).
> > > >
> > > > My attempt at one is attached to bugzilla - SoakLRUMap. I've only run it
> > > > for an hour or so though.
> >
> > I ran something similar for two 3-hour runs with no errors, using a
> > mix of Integer and String keys engineered to get a lot of reuse to
> > happen.  I did lots of shorter runs modifying the the number of
> > buckets, number of threads, and relative frequency of adds / replaces,
> > etc.  It was easy to get the reported errors with synchronization off;
> > but I saw no errors when access was properly synchronized.
> 
> Just for the record, since everyone might not have access to these
> platforms, I saw the same results as Phil while running SoakLRUMap
> overnight on Mac OS X 10.4.3 dual-G4 and dual-G5 hardware with java
> versions 1.4.2_09 and 1.5.0_05.

thanks for the information: it is appreciated :)

the tests on my debian box have been running (so far) for over four days
without an NPE.

maybe we could collate all this on a wiki page?

- robert


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [collections] any objections?

Posted by Michael Heuer <he...@acm.org>.
On Sat, 12 Nov 2005, Phil Steitz wrote:

> On 11/12/05, robert burrell donkin <ro...@blueyonder.co.uk> wrote:
> > On Sat, 2005-11-12 at 13:44 +0000, Stephen Colebourne wrote:
> > > robert burrell donkin wrote:
> > > > has anyone run a long stress test?
> > > >
> > > > if not, i'm willing to code up something and set it running on my debian
> > > > box for a few days. i'd appreciate a second pair of eyes on the code (to
> > > > avoid mistakes).
> > >
> > > My attempt at one is attached to bugzilla - SoakLRUMap. I've only run it
> > > for an hour or so though.
>
> I ran something similar for two 3-hour runs with no errors, using a
> mix of Integer and String keys engineered to get a lot of reuse to
> happen.  I did lots of shorter runs modifying the the number of
> buckets, number of threads, and relative frequency of adds / replaces,
> etc.  It was easy to get the reported errors with synchronization off;
> but I saw no errors when access was properly synchronized.

Just for the record, since everyone might not have access to these
platforms, I saw the same results as Phil while running SoakLRUMap
overnight on Mac OS X 10.4.3 dual-G4 and dual-G5 hardware with java
versions 1.4.2_09 and 1.5.0_05.

   michael


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [collections] LRUBug

Posted by robert burrell donkin <ro...@blueyonder.co.uk>.
On Tue, 2005-11-22 at 10:53 +0000, sebb wrote:
> On 22/11/05, Phil Steitz <ph...@gmail.com> wrote:
> > On 11/21/05, Stephen Colebourne <sc...@btopenworld.com> wrote:
> > > robert burrell donkin wrote:
> > > > i've been running the SOAK tests for over 7 days now without a problem.
> 
> Just to report that I have run the synchronized soak test using 5
> threads and 10,000,000 loops on
> 
> AlphaServer 4X00 5/400 4MB, 4 CPUs
> 
> Running
> OpenVMS 7.3-1
> 
> java version "1.4.0"
> Java(TM) 2 Runtime Environment, Standard Edition
> Fast VM (build 1.4.0-1, build J2SDK.v.1.4.0:01/10/2003-09:47, native
> threads, jit_140)
> 
> This took about 8 minutes.

thanks :)

- robert


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [collections] LRUBug

Posted by sebb <se...@gmail.com>.
On 22/11/05, Phil Steitz <ph...@gmail.com> wrote:
> On 11/21/05, Stephen Colebourne <sc...@btopenworld.com> wrote:
> > robert burrell donkin wrote:
> > > i've been running the SOAK tests for over 7 days now without a problem.

Just to report that I have run the synchronized soak test using 5
threads and 10,000,000 loops on

AlphaServer 4X00 5/400 4MB, 4 CPUs

Running
OpenVMS 7.3-1

java version "1.4.0"
Java(TM) 2 Runtime Environment, Standard Edition
Fast VM (build 1.4.0-1, build J2SDK.v.1.4.0:01/10/2003-09:47, native
threads, jit_140)

This took about 8 minutes.

The unsynchronized test fails rapidly with just 2 threads:

java.lang.NullPointerException
        at org.apache.commons.collections.map.AbstractLinkedMap.removeEntry(AbstractLinkedMap.java:292)
        at org.apache.commons.collections.map.AbstractHashedMap.removeMapping(AbstractHashedMap.java:542)
        at org.apache.commons.collections.map.AbstractHashedMap.remove(AbstractHashedMap.java:324)
        at SoakLRUMapUn.soak(SoakLRUMapUn.java:69)
        at SoakLRUMapUn.run(SoakLRUMapUn.java:80)
        at java.lang.Thread.run(Thread.java:536)

HTH.
S.

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [collections] LRUBug

Posted by Phil Steitz <ph...@gmail.com>.
On 11/21/05, Stephen Colebourne <sc...@btopenworld.com> wrote:
> robert burrell donkin wrote:
> > i've been running the SOAK tests for over 7 days now without a problem.
> > if there is a bug in the collections code (as opposed to problems with
> > synchronization) then i don't think that it'll be discovered by those
> > tests. i can't see any reason for the code to fail when correctly
> > synchronized. so, i think it'd be best release as is and hope that (if
> > the problem exists) someone will donate some production code we can
> > run.
> +1

+1 from me as well.  One day, when someone has some time and energy,
it would be great to document how the data structures in LRUMap and
its superclasses work in the class javadocs.   IIUC, there really is
no way that the NPEs reported can happen without modifying keys or
unsynchronized concurrent access.
>
>
> > what is becoming clear to me is that a number of users have been
> > confused by the need to synchronize the maps. i've added some
> > clarifications to the javadocs for LRUMap.
> > would it be a good idea (for me) to add explicit notes about the need to
> > synchronize maps to other javadocs? and to the user guide?
> I've added a note to all relevant map javadocs (slightly amended from
> your text. It would be good if you could add to the user guide.
>
>
> > BTW i get problems with jdiff (no CVSROOT) when i try maven
> > site:generate any tips?
> Perhaps try the latest jdiff plugin? I have 1.5 and maven jdiff works.

Yes, subversion support was added in version 1.5 of the jdiff plugin.

Phil

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [collections] LRUBug

Posted by robert burrell donkin <ro...@blueyonder.co.uk>.
On Mon, 2005-11-21 at 22:43 +0000, Stephen Colebourne wrote:
> robert burrell donkin wrote:

<snip>

> > what is becoming clear to me is that a number of users have been
> > confused by the need to synchronize the maps. i've added some
> > clarifications to the javadocs for LRUMap. 
> > would it be a good idea (for me) to add explicit notes about the need to
> > synchronize maps to other javadocs? and to the user guide?
> I've added a note to all relevant map javadocs (slightly amended from 
> your text. It would be good if you could add to the user guide.

i've committed a first cut. it probably need some more work so feel free
to dive in with improvements...

i also changes the way the information is structured a little. feel free
to revert or improve.

> > BTW i get problems with jdiff (no CVSROOT) when i try maven
> > site:generate any tips?
> Perhaps try the latest jdiff plugin? I have 1.5 and maven jdiff works.

thanks - that did it :)

- robert


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [collections] LRUBug

Posted by Stephen Colebourne <sc...@btopenworld.com>.
robert burrell donkin wrote:
> i've been running the SOAK tests for over 7 days now without a problem.
> if there is a bug in the collections code (as opposed to problems with
> synchronization) then i don't think that it'll be discovered by those
> tests. i can't see any reason for the code to fail when correctly
> synchronized. so, i think it'd be best release as is and hope that (if
> the problem exists) someone will donate some production code we can
> run.  
+1


> what is becoming clear to me is that a number of users have been
> confused by the need to synchronize the maps. i've added some
> clarifications to the javadocs for LRUMap. 
> would it be a good idea (for me) to add explicit notes about the need to
> synchronize maps to other javadocs? and to the user guide?
I've added a note to all relevant map javadocs (slightly amended from 
your text. It would be good if you could add to the user guide.


> BTW i get problems with jdiff (no CVSROOT) when i try maven
> site:generate any tips?
Perhaps try the latest jdiff plugin? I have 1.5 and maven jdiff works.

Stephen


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [collections] LRUBug

Posted by robert burrell donkin <ro...@blueyonder.co.uk>.
On Sun, 2005-11-13 at 16:38 +0000, Stephen Colebourne wrote:
> >>i would recommend keeping the bugzilla open but adding a comment
> >>requesting that users ensure that the code is synchronized properly and
> >>that they list the full environment (JVM and platform) plus information
> >>about the keys they are using (and ideally a soak tests we can run).
> >>that may give us enough information to track down where the problem
> >>lies.
> > 
> > 
> > +1, plus include additional debugging code in release.
> 
> I have checked in my extra-info on error 'debug' version into SVN. As we 
> can't reproduce, I agree that we have to move towards a release as is.

i've been running the SOAK tests for over 7 days now without a problem.
if there is a bug in the collections code (as opposed to problems with
synchronization) then i don't think that it'll be discovered by those
tests. i can't see any reason for the code to fail when correctly
synchronized. so, i think it'd be best release as is and hope that (if
the problem exists) someone will donate some production code we can
run.  

what is becoming clear to me is that a number of users have been
confused by the need to synchronize the maps. i've added some
clarifications to the javadocs for LRUMap. 

would it be a good idea (for me) to add explicit notes about the need to
synchronize maps to other javadocs? and to the user guide?
 
BTW i get problems with jdiff (no CVSROOT) when i try maven
site:generate any tips?

- robert 


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [collections] LRUBug

Posted by Stephen Colebourne <sc...@btopenworld.com>.
>>i would recommend keeping the bugzilla open but adding a comment
>>requesting that users ensure that the code is synchronized properly and
>>that they list the full environment (JVM and platform) plus information
>>about the keys they are using (and ideally a soak tests we can run).
>>that may give us enough information to track down where the problem
>>lies.
> 
> 
> +1, plus include additional debugging code in release.

I have checked in my extra-info on error 'debug' version into SVN. As we 
can't reproduce, I agree that we have to move towards a release as is.

Stephen

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [collections] any objections?

Posted by Phil Steitz <ph...@gmail.com>.
On 11/12/05, robert burrell donkin <ro...@blueyonder.co.uk> wrote:
> On Sat, 2005-11-12 at 13:44 +0000, Stephen Colebourne wrote:
> > robert burrell donkin wrote:
> > > has anyone run a long stress test?
> > >
> > > if not, i'm willing to code up something and set it running on my debian
> > > box for a few days. i'd appreciate a second pair of eyes on the code (to
> > > avoid mistakes).
> >
> > My attempt at one is attached to bugzilla - SoakLRUMap. I've only run it
> > for an hour or so though.

I ran something similar for two 3-hour runs with no errors, using a
mix of Integer and String keys engineered to get a lot of reuse to
happen.  I did lots of shorter runs modifying the the number of
buckets, number of threads, and relative frequency of adds / replaces,
etc.  It was easy to get the reported errors with synchronization off;
but I saw no errors when access was properly synchronized.

>
> great. it's a simple test but then again, none of the problems were
> reported with obscure keys. i've set off two processes on my debian box
> one running the single thread, one the synchronised multiple thread
> variant.
>
> > I did wonder if this might be some kind of double checked locking effect
> > only seen on multi-processor systems.
>
> otis reports that he was using a beta JVM.
>
> the reports (or at least those who are using correctly synchronized
> versions of the map) coincide with the time when 1.5 JDKs were in beta.
> wonder whether it was some sort of bug in the synchronization code that
> was later fixed by sun.
>
> my debian box is single processor and i don't have access to a double
> processor machine. it's on constantly so i can leave the tests running
> for a few days. i'll report back if there are any NPEs.
>
> i suggest that if there are no NPEs in the few days, we work on the
> assumption that it's some kind of issue in the synchronization code
> (rather than the collection code). so, we then document and add a
> warning to the release notes before moving towards shipping the
> release.

+1
>
> i would recommend keeping the bugzilla open but adding a comment
> requesting that users ensure that the code is synchronized properly and
> that they list the full environment (JVM and platform) plus information
> about the keys they are using (and ideally a soak tests we can run).
> that may give us enough information to track down where the problem
> lies.

+1, plus include additional debugging code in release.
>
> - robert
>
>
> ---------------------------------------------------------------------
> 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] any objections?

Posted by robert burrell donkin <ro...@blueyonder.co.uk>.
On Sat, 2005-11-12 at 13:44 +0000, Stephen Colebourne wrote:
> robert burrell donkin wrote:
> > has anyone run a long stress test?
> > 
> > if not, i'm willing to code up something and set it running on my debian
> > box for a few days. i'd appreciate a second pair of eyes on the code (to
> > avoid mistakes).
> 
> My attempt at one is attached to bugzilla - SoakLRUMap. I've only run it 
> for an hour or so though.

great. it's a simple test but then again, none of the problems were
reported with obscure keys. i've set off two processes on my debian box
one running the single thread, one the synchronised multiple thread
variant.

> I did wonder if this might be some kind of double checked locking effect 
> only seen on multi-processor systems.

otis reports that he was using a beta JVM.

the reports (or at least those who are using correctly synchronized
versions of the map) coincide with the time when 1.5 JDKs were in beta.
wonder whether it was some sort of bug in the synchronization code that
was later fixed by sun.

my debian box is single processor and i don't have access to a double
processor machine. it's on constantly so i can leave the tests running
for a few days. i'll report back if there are any NPEs.

i suggest that if there are no NPEs in the few days, we work on the
assumption that it's some kind of issue in the synchronization code
(rather than the collection code). so, we then document and add a
warning to the release notes before moving towards shipping the
release. 

i would recommend keeping the bugzilla open but adding a comment
requesting that users ensure that the code is synchronized properly and
that they list the full environment (JVM and platform) plus information
about the keys they are using (and ideally a soak tests we can run).
that may give us enough information to track down where the problem
lies.

- robert 


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [collections] any objections?

Posted by Stephen Colebourne <sc...@btopenworld.com>.
robert burrell donkin wrote:
> has anyone run a long stress test?
> 
> if not, i'm willing to code up something and set it running on my debian
> box for a few days. i'd appreciate a second pair of eyes on the code (to
> avoid mistakes).

My attempt at one is attached to bugzilla - SoakLRUMap. I've only run it 
for an hour or so though.

I did wonder if this might be some kind of double checked locking effect 
only seen on multi-processor systems.

Stephen

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [collections] any objections?

Posted by robert burrell donkin <ro...@blueyonder.co.uk>.
On Fri, 2005-11-11 at 00:18 +0000, Stephen Colebourne wrote:
> robert burrell donkin wrote:

<snip>

> > is it time to take seriously the possibility of a bug in synchronisation
> > being an explanation?
> I've added a comment to bugzilla.

has anyone run a long stress test?

if not, i'm willing to code up something and set it running on my debian
box for a few days. i'd appreciate a second pair of eyes on the code (to
avoid mistakes).

- robert


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


RE: [collections] any objections?

Posted by James Carman <ja...@carmanconsulting.com>.
Do you think that the aforementioned "wrapper" classes (TimeoutBuffer and
BoundedBuffer) are candidates for inclusion in the release?

-----Original Message-----
From: Stephen Colebourne [mailto:scolebourne@btopenworld.com] 
Sent: Thursday, November 10, 2005 7:19 PM
To: Jakarta Commons Developers List
Subject: Re: [collections] any objections?

robert burrell donkin wrote:
>>The removeIndex is the index of the hash bucket, not the hash code. This 
>>bit of code is simply trying to find the entry before that we want to 
>>remove, where we already know the entry we want to remove. 
> 
> got that bit but missed the use of header to store order links to
> entries. header is the start of a circular buffer used to store the
> entries in order, right?
The header is the start of a linked list which maintains the LRU order. 
This runs separately to the bucket's next field.


>>None of this 
>>requires us to check using equals().
> 
> 
> true that wasn't the path i was travelling down. i was wondering whether
> the bucket could ever be null (thus producing a NPE) but it can't be if
> there is an entry is still in the map. doesn't seem to be any easy way
> that it could happen given appropriate synchronisation. the other
> candidate is for loop to become null but this shouldn't happen, should
> it?
No, there seems to be no way to get this

> is it time to take seriously the possibility of a bug in synchronisation
> being an explanation?
I've added a comment to bugzilla.


I think we should give this another couple of weeks and then release 
with my additional semi-debugging statements. There is too much other 
stuff that needs releasing, and collections is way way overdue.

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] any objections?

Posted by Stephen Colebourne <sc...@btopenworld.com>.
robert burrell donkin wrote:
>>The removeIndex is the index of the hash bucket, not the hash code. This 
>>bit of code is simply trying to find the entry before that we want to 
>>remove, where we already know the entry we want to remove. 
> 
> got that bit but missed the use of header to store order links to
> entries. header is the start of a circular buffer used to store the
> entries in order, right?
The header is the start of a linked list which maintains the LRU order. 
This runs separately to the bucket's next field.


>>None of this 
>>requires us to check using equals().
> 
> 
> true that wasn't the path i was travelling down. i was wondering whether
> the bucket could ever be null (thus producing a NPE) but it can't be if
> there is an entry is still in the map. doesn't seem to be any easy way
> that it could happen given appropriate synchronisation. the other
> candidate is for loop to become null but this shouldn't happen, should
> it?
No, there seems to be no way to get this

> is it time to take seriously the possibility of a bug in synchronisation
> being an explanation?
I've added a comment to bugzilla.


I think we should give this another couple of weeks and then release 
with my additional semi-debugging statements. There is too much other 
stuff that needs releasing, and collections is way way overdue.

Stephen

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [collections] any objections?

Posted by robert burrell donkin <ro...@blueyonder.co.uk>.
On Wed, 2005-11-09 at 00:38 +0000, Stephen Colebourne wrote:
> robert burrell donkin wrote:
> > On Sat, 2005-11-05 at 10:39 -0700, Phil Steitz wrote:
> > the reported issues seem only to occur when synchronizedMap is used. has
> > anyone looked for a pattern in JVM versions?
> No, not yet
> 
> > or tried to replicate using a multi-threaded test rig? 
> Bugzilla contains a basic multi-thread test
> 
> > starting with the symptoms: if there is problem, it's not common and
> > occurs only when the map is full. AIUI the map is of a fixed maximum
> > size and when full the oldest entry is discarded and the space reused.
> Yes
> 
> > the NPE occurs in LRUMap.reuseMapping which accords with the symptoms
> > (this code is only executed when full). so, there is no reason not to
> > focus on this method first.
> The bug report also has an error in addMapping
> 
> > i may have missed something but i don't think that this isn't a strict
> > LRU implementation: rather, it removes the last recently used entry with
> > the same hashcode. here's the code:
> > 
> >         int removeIndex = hashIndex(entry.hashCode, data.length);
> >         HashEntry loop = data[removeIndex];
> >         HashEntry previous = null;
> >         while (loop != entry) {
> >             previous = loop;
> >             loop = loop.next;
> >         }
> The removeIndex is the index of the hash bucket, not the hash code. This 
> bit of code is simply trying to find the entry before that we want to 
> remove, where we already know the entry we want to remove. 

got that bit but missed the use of header to store order links to
entries. header is the start of a circular buffer used to store the
entries in order, right?

> None of this 
> requires us to check using equals().

true that wasn't the path i was travelling down. i was wondering whether
the bucket could ever be null (thus producing a NPE) but it can't be if
there is an entry is still in the map. doesn't seem to be any easy way
that it could happen given appropriate synchronisation. the other
candidate is for loop to become null but this shouldn't happen, should
it?

is it time to take seriously the possibility of a bug in synchronisation
being an explanation?

- robert


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [collections] any objections?

Posted by Stephen Colebourne <sc...@btopenworld.com>.
robert burrell donkin wrote:
> On Sat, 2005-11-05 at 10:39 -0700, Phil Steitz wrote:
> the reported issues seem only to occur when synchronizedMap is used. has
> anyone looked for a pattern in JVM versions?
No, not yet

> or tried to replicate using a multi-threaded test rig? 
Bugzilla contains a basic multi-thread test

> starting with the symptoms: if there is problem, it's not common and
> occurs only when the map is full. AIUI the map is of a fixed maximum
> size and when full the oldest entry is discarded and the space reused.
Yes

> the NPE occurs in LRUMap.reuseMapping which accords with the symptoms
> (this code is only executed when full). so, there is no reason not to
> focus on this method first.
The bug report also has an error in addMapping

> i may have missed something but i don't think that this isn't a strict
> LRU implementation: rather, it removes the last recently used entry with
> the same hashcode. here's the code:
> 
>         int removeIndex = hashIndex(entry.hashCode, data.length);
>         HashEntry loop = data[removeIndex];
>         HashEntry previous = null;
>         while (loop != entry) {
>             previous = loop;
>             loop = loop.next;
>         }
The removeIndex is the index of the hash bucket, not the hash code. This 
bit of code is simply trying to find the entry before that we want to 
remove, where we already know the entry we want to remove. None of this 
requires us to check using equals().

Stephen

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [collections] any objections?

Posted by robert burrell donkin <ro...@blueyonder.co.uk>.
On Sat, 2005-11-05 at 10:39 -0700, Phil Steitz wrote:
> Big +1.  Agree with Stephen and share frustration over 32573.  I have
> spent quite a few hours trying to replicate or "prove" that the only
> way to make this happen is via unsynchronized access.  

proving that would be tough :-/

the reported issues seem only to occur when synchronizedMap is used. has
anyone looked for a pattern in JVM versions?

or tried to replicate using a multi-threaded test rig? 

> Another set of eyes on that issue would be great.  

i can't claim any deep understanding of the code but sometimes
discussions can throw up a solution. please jump in and correct my
misunderstandings...

starting with the symptoms: if there is problem, it's not common and
occurs only when the map is full. AIUI the map is of a fixed maximum
size and when full the oldest entry is discarded and the space reused.

the NPE occurs in LRUMap.reuseMapping which accords with the symptoms
(this code is only executed when full). so, there is no reason not to
focus on this method first.

i may have missed something but i don't think that this isn't a strict
LRU implementation: rather, it removes the last recently used entry with
the same hashcode. here's the code:

        int removeIndex = hashIndex(entry.hashCode, data.length);
        HashEntry loop = data[removeIndex];
        HashEntry previous = null;
        while (loop != entry) {
            previous = loop;
            loop = loop.next;
        }

this is the code suspected to throw the NPE.  

if this is not a strict LRU then i suspect that loop could legitimately
be null after some entries are reused when the hashCode's are not evenly
distributed. this would be consistent with the symptoms reported and may
be possible to unit test (with difficulty).

does this seem possible?

- robert


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [collections] any objections?

Posted by Stephen Colebourne <sc...@btopenworld.com>.
robert burrell donkin wrote:
> had it in mind just to add some more material to a few classes i was
> using yesterday.

Well, all improvements are welcome!

Stephen

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [collections] any objections?

Posted by robert burrell donkin <ro...@blueyonder.co.uk>.
On Sat, 2005-11-05 at 10:39 -0700, Phil Steitz wrote:
> Big +1.  Agree with Stephen and share frustration over 32573.  I have
> spent quite a few hours trying to replicate or "prove" that the only
> way to make this happen is via unsynchronized access.  Another set of
> eyes on that issue would be great.  

issues.apache.org is dead ATM but i'll try to take a look when it's up
again...

> That code could also use some doco
> improvements, so if you don't mind starting there...;-)

the docs already look pretty good for that class :)

had it in mind just to add some more material to a few classes i was
using yesterday. my documentation priority is the foundation
infrastructure stuff ATM (though i haven't managed too much on that
recently) and there's still lots to be done there. 

- robert


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [collections] any objections?

Posted by Phil Steitz <ph...@gmail.com>.
Big +1.  Agree with Stephen and share frustration over 32573.  I have
spent quite a few hours trying to replicate or "prove" that the only
way to make this happen is via unsynchronized access.  Another set of
eyes on that issue would be great.  That code could also use some doco
improvements, so if you don't mind starting there...;-)

Thanks!

Phil

On 11/5/05, Stephen Colebourne <sc...@btopenworld.com> wrote:
> robert burrell donkin wrote:
> > i'd like to dive in with some improvements to the javadocs. i'm going
> > add myself to the list of committers and get started sometime soonish.
> +1
>
> > so now would be a good time for people to jump in with any objections,
> > comments or warnings...
> Apart from LRUMap's problems, [collections] is about ready for 3.2...
>
> 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] any objections?

Posted by Stephen Colebourne <sc...@btopenworld.com>.
robert burrell donkin wrote:
> i'd like to dive in with some improvements to the javadocs. i'm going
> add myself to the list of committers and get started sometime soonish. 
+1

> so now would be a good time for people to jump in with any objections,
> comments or warnings...
Apart from LRUMap's problems, [collections] is about ready for 3.2...

Stephen

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org