I think an alternative approach to the one here would be to use a global
ReferenceQueue and a subclass of WeakReference that has a pointer back
to the Logger or LogNode that contains this WeakReference. Then, in the
cases where you currently check for stale entries, you could simply poll
the ReferenceQueue and visit any Logger or LogNode that it is telling
you has a cleared WeakReference.
There are a couple of subtle points, though. First, you'll need to be
careful about synchronization before clearing WeakReferences, since the
Logger you're being told about isn't necessarily related to the current
one. Logger.kids is protected by a global lock treeLock but
LogManager.loggers is protected by the LogManager's lock, so when
logManager1.addLogger detects that a logger belonging to logManager2 has
a stale WeakReference, it needs to drop the logManager1 lock and acquire
the logManager2 one before scanning the reference list. (This means that
addLogger has to change from being a synchronized method to have a big
synchronized(this) block with a non-synchronized tail-end.)
Second, since apparently the user can introduce loops in the Logger
hierarchy, the reference from the WeakReference subclass back to the
parent Logger will itself need to be a WeakReference.
Nevertheless, I think this approach is likely to be less complicated
than the proposed one, notably because there's no need to protect
against infinite recursion; and the performance concerns should be less
because you only look for cleared WeakReferences when you know they are
there, and you know where to look.
Éamonn
Alan Bateman wrote:
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.
