commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Brian Stansberry <bes_commons_...@yahoo.com>
Subject Re: [logging]: WeakHashtable
Date Mon, 15 Nov 2004 04:12:36 GMT
I've attached a patch that adds a couple more tests
and fixes a problem found by one of them.  New test
LogFactoryTest.testHoldsFactories() shows that
WeakHashtable does not keep a LogFactory from being
gc'ed even if the ClassLoader associated with it is
still alive.  So, calls to LogFactory.getFactory()
result in new factories being created.  The patch to
WeakHashtable is largely designed to fix that.

The patched WeakHashtable holds values in the table
until the WeakReference to the associated ClassLoader
is removed, even if the classloader itself has been
gc'ed.  Because of this, the potential amount of
garbage left in the table is greater than it was
before.  The patch partly tries to remedy that by
doing a purge before each rehash.

The patch also switches the purge mechanism to one
that uses a ReferenceQueue.  This should be more
performant, as it allows the purging process to only
deal with items (if any) that definitely need to be
removed from the hashtable, rather than iterating
through all entries in the map.  ReferenceQueue.poll()
itself is quite fast, basically consisting of popping
off the first element in a linked list.

In this patch the way LogFactories are kept from being
dropped from the hashtable is not ideal.  Basically
the keys in the map hold hard references to the
LogFactories, keeping the WeakReferences to the
LogFactories from being cleared.  This approach is a
leftover remnant of a failed attempt on my part at
getting the hashtable itself to clear unneeded
factories without the need for a call to purge().  It
would be much cleaner to just have the hashtable hold
normal hard references to the LogFactories.  I didn't
include such a change in this patch as 1) it may have
made the patch overly complicated, and 2) I didn't
have time ;-)  If the powers that be agree that the
LogFactories should be held directly by the Hashtable,
I would be happy to create another patch.

(Also, there's some funky stuff in the test cases
where I try to handle OutOfMemoryError.  It works on
my environment (Eclipse 3.0, Sun JDK 1.4.2_03), but if
others have thoughts about this, they would be much
appreciated).

Best,
Brian

--- Brian Stansberry <bes_commons_dev@yahoo.com>
wrote:

> --- robert burrell donkin  wrote:
> 
> > 
> > On 11 Nov 2004, at 07:40, Brian Stansberry wrote:>
> 
> > >  A couple things occurred to me as I looked.
> > >  
> > > 1)  The instances of Referenced are not cleared
> > from the underlying 
> > > table if their underlying references are
> cleared.
> > > 2)  Passing null keys/values to put() does not
> > result in a NPE.
> > > 
> > > One thought on #1 is to make Referenced a
> subclass
> > of WeakReference 
> > > instead of a wrapper.  You can then keep a
> > ReferenceQueue and poll it 
> > > to remove cleared references from the hashtable
> > whenever get() is 
> > > called.  This is similar to what WeakHashMap
> does.
> > 
> > i had a bit of a think about the best way to do
> > this. i think the 
> > approach outlined would be best if this were a
> > general implementation. 
> > in this case, though, the majority of calls are
> > going to be to get with 
> > somewhat less going to put and very few to any
> > others. i can't think of 
> > any occasions when the symantics of put and get
> are
> > influenced by the 
> > presence of extra entries. so i've opted for code
> > that simply purges 
> > entries that have lost their referants which is
> > called at the start of 
> > other interrogative methods. the data returned
> will
> > be more stale than 
> > using a reference queue but i think that
> liveliness
> > for put and get should be improved.
> > 
> Yep, slowing down the critical get() just to sweep
> up
> some dust in the corners makes no sense.
> 
> > i'd be grateful if people would take a look at the
> > code and try to find 
> > any holes in this approach or reasons why using a
> > ReferenceQueue might 
> > improve liveliness (preferably with patches)...
> 
> I was thinking about this and concluded that the
> approach of iterating the Hashtable.entrySet() would
> be faster since you're checking if either the key or
> the value has been cleared.  Using a ReferenceQueue
> for values would force you to use a reverse lookup
> map, which seems inefficient.
> 
> But then I thought, wait, should the values be held
> in
> WeakReferences?  In a typical case where the
> application just calls LogFactory.getLog(), won't
> the
> only reference to the LogFactory instance be the
> value
> in the map?  In this case a lot of calls to getLog()
> will end up going through the getFactory() discovery
> mechanism as the GC keeps clearing the values from
> the
> hashtable.
> 
> 
> Brian
> 
> 
> 		
> __________________________________ 
> Do you Yahoo!? 
> Check out the new Yahoo! Front Page. 
> www.yahoo.com 
>  
> 
> 
>
---------------------------------------------------------------------
> To unsubscribe, e-mail:
> commons-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail:
> commons-dev-help@jakarta.apache.org
> 
> 




		
__________________________________ 
Do you Yahoo!? 
Check out the new Yahoo! Front Page. 
www.yahoo.com 
 

Mime
View raw message