<html>
<head>
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
</head>
<body style="background-color: rgb(255, 255, 255); color: rgb(0, 0,
0);" text="#000000" bgcolor="#FFFFFF">
Hi Kim,<br>
<br>
Thanks again for looking into changed code. Some comments to your
comments for easier understanding of my code (when you have time)...<br>
<br>
<div class="moz-cite-prefix">On 06/26/2015 11:36 PM, Kim Barrett
wrote:<br>
</div>
<blockquote
cite="mid:73CD8857-E400-4F30-9651-39ECBE19CC51@oracle.com"
type="cite"><!--[if !IE]><DIV style="border-left: 2px solid #009900; border-right: 2px solid #009900; padding: 0px 15px; margin: 2px 0px;"><![endif]-->
<pre wrap="">On Jun 22, 2015, at 3:33 AM, Peter Levart <a class="moz-txt-link-rfc2396E" href="mailto:peter.levart@gmail.com"><peter.levart@gmail.com></a> wrote:
</pre>
<blockquote type="cite"><!--[if !IE]><DIV style="border-left: 2px solid #009900; border-right: 2px solid #009900; padding: 0px 15px; margin: 2px 0px;"><![endif]-->
<pre wrap="">Hi Kim,
</pre>
<!--[if !IE]></DIV><![endif]--></blockquote>
<pre wrap="">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.
</pre>
<blockquote type="cite"><!--[if !IE]><DIV style="border-left: 2px solid #009900; border-right: 2px solid #009900; padding: 0px 15px; margin: 2px 0px;"><![endif]-->
<pre wrap="">On 06/09/2015 07:44 AM, Peter Levart wrote:
</pre>
<blockquote type="cite"><!--[if !IE]><DIV style="border-left: 2px solid #009900; border-right: 2px solid #009900; padding: 0px 15px; margin: 2px 0px;"><![endif]-->
<pre wrap="">On 06/09/2015 04:03 AM, Kim Barrett wrote:
</pre>
<blockquote type="cite"><!--[if !IE]><DIV style="border-left: 2px solid #009900; border-right: 2px solid #009900; padding: 0px 15px; margin: 2px 0px;"><![endif]-->
<pre wrap="">On May 31, 2015, at 3:32 PM, Peter Levart <a class="moz-txt-link-rfc2396E" href="mailto:peter.levart@gmail.com"><peter.levart@gmail.com></a>
wrote:
</pre>
<blockquote type="cite"><!--[if !IE]><DIV style="border-left: 2px solid #009900; border-right: 2px solid #009900; padding: 0px 15px; margin: 2px 0px;"><![endif]-->
<pre wrap="">So, for the curious, here's the improved prototype:
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~plevart/misc/JEP132/ReferenceHandling/webrev.02/">http://cr.openjdk.java.net/~plevart/misc/JEP132/ReferenceHandling/webrev.02/</a>
</pre>
<!--[if !IE]></DIV><![endif]--></blockquote>
<pre wrap="">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. […]
</pre>
<!--[if !IE]></DIV><![endif]--></blockquote>
<pre wrap="">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.
</pre>
<!--[if !IE]></DIV><![endif]--></blockquote>
<pre wrap="">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.
</pre>
<!--[if !IE]></DIV><![endif]--></blockquote>
<pre wrap="">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.</pre>
<!--[if !IE]></DIV><![endif]--></blockquote>
<br>
That's exactly my understanding too.<br>
<br>
<blockquote
cite="mid:73CD8857-E400-4F30-9651-39ECBE19CC51@oracle.com"
type="cite"><!--[if !IE]><DIV style="border-left: 2px solid #009900; border-right: 2px solid #009900; padding: 0px 15px; margin: 2px 0px;"><![endif]-->
<pre wrap="">
</pre>
<blockquote type="cite"><!--[if !IE]><DIV style="border-left: 2px solid #009900; border-right: 2px solid #009900; padding: 0px 15px; margin: 2px 0px;"><![endif]-->
<pre wrap=""> Here's the modified prototype that uses 'discovered' field to form chunks of references:
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~plevart/misc/JEP132/ReferenceHandling/webrev.05/">http://cr.openjdk.java.net/~plevart/misc/JEP132/ReferenceHandling/webrev.05/</a>
[…]
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
</pre>
<!--[if !IE]></DIV><![endif]--></blockquote>
<pre wrap="">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).</pre>
<!--[if !IE]></DIV><![endif]--></blockquote>
<br>
Yes, you might be confusing 'discovered' field with 'next' field.
Some facts about 'discovered' field:<br>
<br>
- it starts as null. Used by GC to form internal lists of discovered
references which *are* terminated with a self-loop.<br>
- 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.<br>
- therefore last element of the pending/discovered chain has
discovered == null (unlike 'next' field in pending or last enqueued
reference)<br>
- 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.<br>
<br>
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.<br>
<br>
I think this does not break any invariants expected by GC to be
maintained.<br>
<br>
<blockquote
cite="mid:73CD8857-E400-4F30-9651-39ECBE19CC51@oracle.com"
type="cite"><!--[if !IE]><DIV style="border-left: 2px solid #009900; border-right: 2px solid #009900; padding: 0px 15px; margin: 2px 0px;"><![endif]-->
<pre wrap="">
</pre>
<blockquote type="cite"><!--[if !IE]><DIV style="border-left: 2px solid #009900; border-right: 2px solid #009900; padding: 0px 15px; margin: 2px 0px;"><![endif]-->
<pre wrap="">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. […]
</pre>
<!--[if !IE]></DIV><![endif]--></blockquote>
<pre wrap="">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.</pre>
<!--[if !IE]></DIV><![endif]--></blockquote>
<br>
Using an internal instance of FJPool for example like in the
presented prototype? I have been thinking of alternatives:<br>
<br>
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.<br>
<br>
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.<br>
<br>
<blockquote
cite="mid:73CD8857-E400-4F30-9651-39ECBE19CC51@oracle.com"
type="cite"><!--[if !IE]><DIV style="border-left: 2px solid #009900; border-right: 2px solid #009900; padding: 0px 15px; margin: 2px 0px;"><![endif]-->
<pre wrap="">
</pre>
<blockquote type="cite"><!--[if !IE]><DIV style="border-left: 2px solid #009900; border-right: 2px solid #009900; padding: 0px 15px; margin: 2px 0px;"><![endif]-->
<pre wrap="">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
</pre>
<!--[if !IE]></DIV><![endif]--></blockquote>
<pre wrap="">See <a class="moz-txt-link-freetext" href="https://bugs.openjdk.java.net/browse/JDK-8080225">https://bugs.openjdk.java.net/browse/JDK-8080225</a>
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.</pre>
<!--[if !IE]></DIV><![endif]--></blockquote>
<br>
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.<br>
<br>
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().<br>
<br>
<blockquote
cite="mid:73CD8857-E400-4F30-9651-39ECBE19CC51@oracle.com"
type="cite"><!--[if !IE]><DIV style="border-left: 2px solid #009900; border-right: 2px solid #009900; padding: 0px 15px; margin: 2px 0px;"><![endif]-->
<pre wrap="">
</pre>
<blockquote type="cite"><!--[if !IE]><DIV style="border-left: 2px solid #009900; border-right: 2px solid #009900; padding: 0px 15px; margin: 2px 0px;"><![endif]-->
<pre wrap="">Are any of the features of this prototype interesting for you and worth pursuing further?
</pre>
<!--[if !IE]></DIV><![endif]--></blockquote>
<pre wrap="">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.
</pre>
<!--[if !IE]></DIV><![endif]--></blockquote>
<br>
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 ;-)<br>
<br>
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.<br>
<br>
Regards, Peter<br>
<br>
</body>
</html>