Hi Peter, I found this thread has grown to discuss several relevant issues that can be separated. 1. Assist cleaner to deallocate direct byte buffer (JDK-6857566) 2. Replace direct byte buffer with java.lang.ref.Cleaner 3. java.lang.ref.Cleaner be an interface vs final class 4. Pending reference list lock (JDK-8055232)
Roger has to speak for #3 which I don’t think he comments on that. For #4, working out the VM and library new interface to transfer the pending references definitely has to take more time and prototype. I’m interested in #4 but not sure if this can target in JDK 9 (given that FC in May). I’d like to address #1 to have the allocating thread to invoke cleaning actions to free native memory rather than any cleaning action. #2 is not as critical in my opinion while it’d be nice to get to. One possible option to move forward is to keep Cleaner as is and keep java.nio.Bits to invoke cleaning actions, i.e. webrev.08.part2 except that CleanerFactory will have two special cleaners - one for native memory cleaning and the other for anything else (there isn’t any client in JDK yet). We will see what Panama would provide for timely deallocation and we could replace the fix in Bits with that when’s available. My comments inlined below that are related #1 and #2. > On Mar 28, 2016, at 10:18 AM, Peter Levart <peter.lev...@gmail.com> wrote: > > > But first, let me reply to Mandy's comments... > > >> My uncomfort was the fix for JDK-6857566 - both enqueuing pending ref and >> invoking the cleaning code in an arbitrary thread. >> >> Looking at it again - enqueuing the pending reference is not so much of a >> concern (simply updating the link) but the common cleaner could be used by >> other code that may only expect to be invoked in system thread that’s still >> my concern (thinking of thread locals). >> > > As you'll see in the webrev below, enqueueing is performed solely be > ReferenceHandler thread. Allocating thread(s) just wait for it to do its job. > There's a little synchronization action performed at the end of enqueueing a > chunk of pending references that notifies waiters (allocating threads) so > that they can continue. This actually improves throughput (compared to > helping enqueue Reference(s) one by one) because there's not much actual work > to be done (just swapping pointers) so synchronization dominates. The goal > here is to minimize synchronization among threads and by executing enqueuing > of the whole bunch of pending references in private by a single thread > achieves a reduction in synchronization when lots of Reference(s) are > discovered at once - precisely the situation when it matters. > I understand this and have no issue with this. > OTOH helping the Cleaner thread is beneficial as cleanup actions take time to > execute and this is the easiest way to retry allocation while there's still > chance it will succeed. As the common Cleaner is using InnocuousThread, > cleanup actions can't rely on any thread locals to be preserved from > invocation to invocation anyway - they are cleared after each cleanup action > so each action gets empty thread locals. We could simulate this in threads > that help execute cleanup actions by saving thread-locals to local variables, > clearing thread-locals, executing cleanup action and then restoring > thread-locals from local variables. Mandy, if you think this is important > I'll add such save/clear/restore code to appropriate place. I’m comfortable of running unsafe::freeMemory in allocating thread. That’s why I propose to have a specific cleaner for native memory allocation use that seems to be the simplest approach (but if it turns out changing to Cleaner as as interface - it’s a different question). I can’t speak for NIO if we want to put save/clear/restore logic in java.nio.Bits. Mandy
