Code Review for WeakReference leak in the Logging API (6942989)

Alan Bateman Alan.Bateman at oracle.com
Fri Jun 11 06:18:34 PDT 2010


Daniel D. Daugherty wrote:
> :
>
> Here is the URL for the webrev:
>
>    http://cr.openjdk.java.net/~dcubed/6942989-webrev/0/
>
I went through the changes too.

I agree with the performance concern with expunging stale entries each 
time a LogManager is created. Would it make sense to do this once every 
N times rather than every time? Or maybe keep a timestamp so that you 
limit the number of times your attempt to expunge in some time period? 
(say a maximum of once a minute).

Does LogManager.deleteStaleWeakRefs need to be synchronized with any 
other operations?

I wonder if Logger.kidsCleanupMarker is the best name for this [ kids 
and markers usually involve clean-up but that's not what is meant here 
:-)  ]. Maybe rename to something that includes the word "sentinel"?

Is is necessary to use jmap in the regression test? Would it be simpler 
to have a pure java test that runs with a small heap size? If it leaks 
then it fails with OOME.

-Alan.






More information about the serviceability-dev mailing list