> On Apr 1, 2016, at 4:31 PM, Roger Riggs <roger.ri...@oracle.com> wrote: > > Hi Peter, > > I overlooked the introduction of another nested class (Task) to handle the > cleanup. > But there are too many changes to see which ones solve a single problem. > > Sorry to make more work, but I think we need to go back to the minimum > necessary > change to make progress on this. Omit all of the little cleanups until the end > or do them first and separately. >
+1. That’d really help the reviewers. thanks Mandy > Thanks, Roger > > > > > On 4/1/16 5:51 PM, Roger Riggs wrote: >> Hi Peter, >> >> Thanks for the diffs to look at. >> >> Two observations on the changes. >> >> - The Cleaner instance was intentionally and necessarily different than the >> CleanerImpl to enable >> the CleanerImpl and its thread to terminate if the Cleaner is not longer >> referenced. >> Folding them into a single object breaks that. >> >> Perhaps it is not too bad for ExtendedCleaner to subclass CleanerImpl with >> the cleanup helper/supplier behavior >> and expose itself to Bits. There will be fewer moving parts. There is no >> need for two factory methods for >> ExtendedCleaner unless you are going to use a separate ThreadFactory. >> >> - The Deallocator (and now Allocator) nested classes are identical, and >> there is a separate copy for each >> type derived from the Direct-X-template. But it may not be worth fixing >> until the rest of it is settled to avoid >> more moving parts. >> >> I don't have an opinion on the code changes in Reference, that's different >> kettle of fish. >> >> More next week. >> >> Have a good weekend, Roger >> >> >> On 4/1/2016 12:46 PM, Peter Levart wrote: >>> >>> >>> On 04/01/2016 06:08 PM, Peter Levart wrote: >>>> >>>> >>>> On 04/01/2016 05:18 PM, Peter Levart wrote: >>>>> @Roger: >>>>> >>>>> ... >>>>> >>>>> About entanglement between nio Bits and >>>>> ExtendedCleaner.retryWhileHelpingClean(). It is the same level of >>>>> entanglement as between the DirectByteBuffer constructor and >>>>> Cleaner.register(). In both occasions an action is provided to the >>>>> Cleaner. Cleaner.register() takes a cleanup action and >>>>> ExtendedCleaner.retryWhileHelpingClean() takes a retriable "allocating" >>>>> or "reservation" action. "allocation" or "reservation" is the opposite of >>>>> cleanup. Both methods are encapsulated in the same object because those >>>>> two functions must be coordinated. So I think that collocating them >>>>> together makes sense. What do you think? >>>> >>>> ...to illustrate what I mean, here's a variant that totally untangles Bits >>>> from Cleaner and moves the whole Cleaner interaction into the >>>> DirectByteBuffer itself: >>>> >>>> http://cr.openjdk.java.net/~plevart/jdk9-dev/removeInternalCleaner/webrev.13.part2/ >>>> >>>> >>>> Notice the symmetry between Cleaner.retryWhileHelpingClean : >>>> Cleaner.register and Allocator : Deallocator ? >>>> >>>> >>>> Regards, Peter >>>> >>> >>> And here's also a diff between webrev.12.part2 and webrev.13.part2: >>> >>> http://cr.openjdk.java.net/~plevart/jdk9-dev/removeInternalCleaner/webrev.diff.12to13.part2/ >>> >>> >>> Regards, Peter >>> >> >
