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.
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