> On Aug 13, 2016, at 4:07 PM, Mandy Chung <mandy.ch...@oracle.com> wrote: > > >> On Aug 13, 2016, at 1:20 AM, Peter Levart <peter.lev...@gmail.com> wrote: >> >> Hi Mandy, >> >> On 08/13/2016 01:55 AM, Mandy Chung wrote: >>>> On Aug 8, 2016, at 6:25 PM, Kim Barrett <kim.barr...@oracle.com> >>>> wrote: >>>> >>>> full: >>>> http://cr.openjdk.java.net/~kbarrett/8156500/jdk.04/ >>>> >>>> >>>> http://cr.openjdk.java.net/~kbarrett/8156500/hotspot.04/ >>> This looks very good. >>> >>> Have you considered having JVM_WaitForReferencePendingList method to return >>> the pending list? i.e. make it a blocking version of >>> getAndClearReferencePendingList rather than two operations. >>> >>> waitForReferenceProcessing is really wait for pending reference(s) enqueued >>> (additionally cleaner gets invoked). What do you think to rename this >>> method to “waitForPendingReferencesEnqueued”? >>> >> >> I think the split is intentional. It's a clever way to avoid an otherwise >> inevitable race that could otherwise cause DBB allocating thread to fail >> with OOME prematurely. >> > > Ah I see. I initially read that pendingList doesn’t need to be set within > the synchronized block with processPendingActive flag but such race could > cause DBB to fail with OOME, as you clarified. > > Then I suggest waitForReferencePendingList (and its JVM function) be renamed > to: > boolean awaitReferencePendingList(); > > a blocking version of "boolean hasReferencePendingList()”. Minor difference > but I think it’s little clearer.
Regarding the name, e.g. "waitFor" vs "await", the meaning is the same. If there were a documented or defacto preference for "await" I would feel obliged to rename, but the JDK seems to use both forms in roughly comparable numbers. Regarding the signature and description, recall that waitForReferencePendingList() does not return a value, nor does it need to. So documenting it as a blocking version of hasReferencePendingList() isn't correct. > waitForReferenceProcessing - I’d like the javadoc to include more details or > @implNote for example: > This method returns true to indicate that some pending references have been > enqueued. If there are cleaners, cleaning actions are queued to be invoked > e.g. DirectByteBuffer may be deallocated. > > This would help the readers to understand what this method is intended to do. I was intentionally a little vague in the description; I don't want to be promising too much, and having callers depend on behavior that we might want to change. In particular, it was intentional that there's no mention of Cleaner or any special behavior for them. For example, we might later decide we don't want the reference processing thread to incur the cost of notifying possible waiters and the associated synchronization, and instead let "waiters" return immediately when there is reference processing in progress, on the assumption that the reference processing thread makes rapid progress. In some respects that's probably more similar in effect to what the old code did. Also recall that this function is presently private, accessed by nio.Bits code via SharedSecrets (and via reflection by one test). It might be appropriate to change the access to this function or provide some similar non-private function in the future, at which point the javadoc and possibly the semantics and implementation will need to be carefully examined.
