Peter,
http://cr.openjdk.java.net/~vlivanov/8079205/webrev.01
https://bugs.openjdk.java.net/browse/JDK-8079205
Your Finalizator touches are good. Supplier interface is not needed as
there is a public Reference superclass that can be used for return type
of JavaLangRefAccess.createFinalizator(). You can remove the import for
Supplier in Reference now.
Thanks for spotting that. Will do.
Recent change in sun.misc.Cleaner behavior broke CallSite context
cleanup.
CallSite references context class through a Cleaner to avoid its
unnecessary retention.
The problem is the following: to do a cleanup (invalidate all affected
nmethods) VM needs a pointer to a context class. Until Cleaner is
cleared (and it was a manual action, since Cleaner extends
PhantomReference isn't automatically cleared according to the docs),
VM can extract it from CallSite.context.referent field.
If PhantomReference.referent wasn't cleared by VM when PhantomReference
was equeued, could it happen that the referent pointer was still != null
and the referent object's heap memory was already reclaimed by GC? Is
that what JDK-8071931 is about? (I cant't see the bug - it's internal).
In that respect the Cleaner based solution was broken from the start, as
you did dereference the referent after Cleaner was enqueued. You could
get a != null pointer which was pointing to reclaimed heap. In Java this
dereference is prevented by PhantomReference.get() always returning null
(well, nothing prevents one to read the referent field with reflection
though).
No, the object isn't reclaimed until corresponding reference is != null.
When Cleaner wasn't automatically cleared, it was guaranteed that the
referent is alive when cleanup action happens. That is the assumption
CallSite dependency tracking is based on.
It's not the case anymore when Cleaner is automatically cleared. Cleanup
action can happen when context Class is already GCed. So, I can access
neither the context mirror class nor VM-internal InstanceKlass.
FinalReference(s) are different in that their referent is not reclaimed
while it is still reachable through FinalReference, which means that
finalizable objects must undergo at least two GC cycles, with processing
in the reference handler and finalizer threads inbetween the cycles, to
be reclaimed. PhantomReference(s), on the other hand, can be enqueued
and their referents reclaimed in the same GC cycle, can't they?
Since PhantomReference isn't automatically cleared, GC can't reclaim its
referent until the reference is manually cleared or PhantomReference
becomes unreachable as a result of cleanup action.
So, it's impossible to reclaim it in the same GC cycle (unless it is a
concurrent GC cycle?).
I experimented with moving cleanup logic into VM [1],
What's the downside of that approach? I mean, why is GC-assisted
approach better? Simpler?
IMO the more code on JDK side the better. More safety guarantees are
provided in Java code comparing to native/VM code.
Also, JVM-based approach suffers from the fact it doesn't get prompt
notification when context Class can be GCed. Since it should work with
autocleared references, there's no need in a cleanup action anymore.
By the time cleanup happens, referent can be already GCed.
That's why I switched from Cleaner to WeakReference. When
CallSite.context is cleared ("stale" context), VM has to scan all
nmethods in the code cache to find all affected nmethods.
BTW I had a private discussion with Kim Barrett who suggested an
alternative approach which doesn't require full code cache scan. I plan
to do some prototyping to understand its feasibility, since it requires
non-InstanceKlass-based nmethod dependency tracking machinery.
Best regards,
Vladimir Ivanov
but Peter Levart came up with a clever idea and implemented
FinalReference-based cleaner-like Finalizator. Classes don't have
finalizers, but Finalizator allows to attach a finalization action to
them. And it is guaranteed that the referent is alive when
finalization happens.
Also, Peter spotted another problem with Cleaner-based implementation.
Cleaner cleanup action is strongly referenced, since it is registered
in Cleaner class. CallSite context cleanup action keeps a reference to
CallSite class (it is passed to MHN.invalidateDependentNMethods).
Users are free to extend CallSite and many do so. If a context class
and a call site class are loaded by a custom class loader, such loader
will never be unloaded, causing a memory leak.
Finalizator doesn't suffer from that, since the action is referenced
only from Finalizator instance. The downside is that cleanup action
can be missed if Finalizator becomes unreachable. It's not a problem
for CallSite context, because a context is always referenced from some
CallSite and if a CallSite becomes unreachable, there's no need to
perform a cleanup.
Testing: jdk/test/java/lang/invoke, hotspot/test/compiler/jsr292
Contributed-by: plevart, vlivanov
Best regards,
Vladimir Ivanov
PS: frankly speaking, I would move Finalizator from java.lang.ref to
java.lang.invoke and call it Context, if there were a way to extend
package-private FinalReference from another package :-)
Modules may have an answer for that. FinalReference could be moved to a
package (java.lang.ref.internal or jdk.lang.ref) that is not exported to
the world and made public.
On the other hand, I don't see a reason why FinalReference couldn't be
part of JDK public API. I know it's currently just a private mechanism
to implement finalization which has issues and many would like to see it
gone, but Java has learned to live with it, and, as you see, it can be a
solution to some problems too ;-)
Regards, Peter
[1] http://cr.openjdk.java.net/~vlivanov/8079205/webrev.00
