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
