JEP 132: More-prompt finalization

Sat Jun 27 09:31:30 UTC 2015

Hi Kim,

Thanks again for looking into changed code. Some comments to your 
comments for easier understanding of my code (when you have time)...

On 06/26/2015 11:36 PM, Kim Barrett wrote:
> On Jun 22, 2015, at 3:33 AM, Peter Levart <peter.levart at gmail.com> wrote:
>> Hi Kim,
> Sorry, I just noticed you'd sent an update a few days ago.
>
> I don't have time to look carefully today, but a few quick comments.
>
>> On 06/09/2015 07:44 AM, Peter Levart wrote:
>>> On 06/09/2015 04:03 AM, Kim Barrett wrote:
>>>> On May 31, 2015, at 3:32 PM, Peter Levart <peter.levart at gmail.com>
>>>>   wrote:
>>>>
>>>>> So, for the curious, here's the improved prototype:
>>>>>
>>>>>      
>>>>> http://cr.openjdk.java.net/~plevart/misc/JEP132/ReferenceHandling/webrev.02/
>>>> While perusing the offered code (webrev.02) (not a careful review
>>>> yet), I think I've found a serious bug:
>>>>
>>>> In Reference.java
>>>> in unhookPendingChunk
>>>>   253             // assert r.next == r;
>>>>   254             // move a chunk of pending/discovered references to a
>>>>   255             // temporary local r/next chain
>>>> ...
>>>>   262                 rd.next = r;
>>>>
>>>> I think that assertion suggested at line 253 does not hold, and line
>>>> 262 may damage a queue.
>>>>
>>>> […]
>>>> I'm not sure why unhook is using the next field at all, rather than
>>>> just continuing to use the discovered field.  […]
>>> Ops, you're right. Explicit enqueue-ing is what skipped my mind since I've been 1st working on finalizers and then generalized the solution... I'm afraid to use the 'discovered' field since it is in the domain of VM and I think Java side can only reset it to null while holding the lock. I also would not like to add another field to Reference just to support "chunking". Perhaps the chunk could be formed as a re-usable array of references. Let me think about this some more to figure out how to solve it most elegantly...
>>>
>>> I'll follow-up when I have something.
>> I think I was too afraid to use the 'discovered' field. I now think the 'discovered' field is the right field to use to form "chunks" of pending references. If I read the VM reference handling code correctly, then 'discovered' field is treated as a normal instance field as far as GC is concerned as soon as the Reference is not active any more (reference.next != null). All pending references found by Java code navigating 'pending' chain are already "de-activated" by GC when discovered (reference.next == reference). So it is safe to manipulate the 'discovered' field in Java code.
> Yes, that's what I had in mind.  The collector must treat the
> discovered field that way, because once notified and added to the
> pending list, a reference is mostly out of the hands of the collector
> and the discovered field is just the link for the pending list, and
> from then on is the responsibility of Java code to deal with.

That's exactly my understanding too.

>>   Here's the modified prototype that uses 'discovered' field to form chunks of references:
>>
>>      http://cr.openjdk.java.net/~plevart/misc/JEP132/ReferenceHandling/webrev.05/
>>
>> […]
>> With this arrangement, I get even less CPU overhead when running the benchmark. Most probably because unhooking the chunk of pending references now doesn't involve writes to many fields - just two writes per chunk: the 'discovered' field of the last Reference in chunk is reset to null
> That might be wrong, and the end marker for a list linked through
> discovered is supposed to be a self-loop.  But I may be misremembering
> or confusing with next-linked lists, and don't have time today to
> examine the code.  I remember the discovery code cares about whether
> that field is NULL or not, but it may not matter when the reference
> has been deactivated (next field is non-NULL).

Yes, you might be confusing 'discovered' field with 'next' field. Some 
facts about 'discovered' field:

- it starts as null. Used by GC to form internal lists of discovered 
references which *are* terminated with a self-loop.
- when GC publishes such internal list by hooking it onto static 
'pending' field, it makes sure the list is terminated by null - it also 
de-activates such reference(s) by self-looping their 'next' field at the 
same time.
- therefore last element of the pending/discovered chain has discovered 
== null (unlike 'next' field in pending or last enqueued reference)
- original reference handling code resets 'discovered' field to null as 
soon as it is unhooked from the 'pending' chain - it does unhooking 
one-by-one.

My changes to reference handling code treat 'discovered' field 
similarly. They resets it to null, albeit not in the same order as 
original code. Last element in the chunk of unhooked pending references 
has 'discovered' field reset immediately as soon as the chunk is 
unhooked. Other elements of chunk have the field reset to null as they 
are processed or enqueued.

I think this does not break any invariants expected by GC to be maintained.

>> The prototype contains other things as well - not all of them might be suitable for inclusion in JDK9. They could be selectively removed from it or tailored. But I think they deserve to be mentioned and considered:
>>
>> Using FJPool as a means to execute Finalizer(s) and other j.l.r.Cleaner(s) which enables easy scaling to more than one worker thread with less overhead than ReferenceQueue based processing.
>>
>> Public j.l.r.Cleaner interface. […]
> There is already some ongoing discussion of this kind of thing.  I
> think we should be encouraging folks to transition away from
> finalize() to reference-based cleanup.  But the amount of boiler-plate
> code needed to use references directly makes that not so appealing.  I
> did a little prototyping there, but have handed off to Roger Riggs.
>
> Roger mentioned that some of the feedback he'd received on the little
> prototype I gave him was about the need for some sort of worker thread
> pool for scaling.

Using an internal instance of FJPool for example like in the presented 
prototype?  I have been thinking of alternatives:

ReferenceQueue based processing has one advantage over using a single 
common pool of thread(s). It allows forming different queues for 
different purposes. Each queue can be processed by separate thread(s) so 
they are isolated. While a common pool of threads used for mixed 
clean-up purposes can be more efficient it is also more fragile when 
some clean-up code misbehaves as it affects more references.

An alternative to having a public j.l.r.Cleaner interface presented in 
the prototype which makes it's implementing Reference(s) "active" 
objects processed by a shared pool of threads could be adding 
j.l.r.Reference constructor(s) taking an Executor argument instead of a 
ReferenceQueue. The problem with this approach is that Executor is an 
interface which can be implemented by user and does not shield the sole 
reference handler thread from executing user code. We would not want a 
custom misbehaving Executor to take down the whole JVM by blocking the 
reference handler thread. So perhaps, instead of Executor, the 
additional Reference constructor argument could be a type that prevents 
user from overriding it's task-submission code path, but still allows 
user to customize it in other aspects.

>> Public j.l.r.Finalizator is an example of a FinalReference based j.l.r.Cleaner. While [Phantom|Weak|Soft]Cleaner is more safe since it doesn't allow access to it's referent when fired, some JDK code (File[Input|Output]Stream for example) still uses finalization
> See https://bugs.openjdk.java.net/browse/JDK-8080225
>
> It seems that for some applications finalize-based cleanup has a
> substantial performance cost that is a complete waste.  A closed
> stream still needs to be finalized, delaying reclamation of the memory
> and pushing the associated final-reference through the various lists
> and threads until the finalize() function finally gets called and
> finds it has nothing to do.
>
> PhantomReference-based cleanup allows the phantom reference to be
> deregistered, cleared, and dropped as part of a close-like operation,
> completely eliminating reference processing costs for a closed object.

The j.l.r.Finalizator presented in the prototype is similar in that 
respect. If it was used in File[Input|Output]Stream instead of classical 
finalize() method, it would, in most cases, prevent the object from 
passing reference processing steps. If a FinalReference is clear()ed as 
part of a close-like operation, it is never discovered by GC as a 
pending reference. j.l.r.Finalizator also eliminates the need for VM -> 
Java registration callback, as it is manually registered - usually in 
object's constructor.

I have looked into what it would take to modify 
File[Input|Output]Stream, FileDiscriptor, etc. code to use 
PhantomReference-based cleanup instead of finalize() and it is not easy. 
If I manage to push it to the end I will present it here, but as far as 
I've come, I had to modify code in lots of places including redesigning 
the API of JNI methods so that they are static, etc. We'll see if such 
redesign is still manageable or if it is easier to use a 
j.l.r.Finalizator-like mechanism that still allows clean-up code to 
access the "finalizee" state but also allows FinalReference 
de-registration on close().

>> Are any of the features of this prototype interesting for you and worth pursuing further?
> This mostly appears to be a question for core-libs; that's where the
> proposed changes are located.  GC is involved mostly to ensure the
> internal APIs between the collector and reference processing are
> maintained.
>
> My personal opinion is that we should be working toward a day when we
> can remove the finalize function, rather than trying to make the
> infrastructure around it faster.  But clearly that day won't be soon,
> if ever.
>
>

The infrastructure is shared for all Reference kinds. Improving it 
impacts all reference processing, not only finalization. If it makes 
finalization faster too, it can be viewed as just a coincidence ;-)

But I agree. We should move internal code away from finalization and 
also give users viable alternatives that would make finalize() method 
obsolete one day.

Regards, Peter