> On Mar 23, 2016, at 3:33 PM, Peter Levart <peter.lev...@gmail.com> wrote:
> 
> Hi Kim,
> 
> On 03/23/2016 07:55 PM, Kim Barrett wrote:
>>> On Mar 23, 2016, at 10:02 AM, Peter Levart <peter.lev...@gmail.com>
>>>  wrote:
>>> ...so I checked what it would be needed if there was such 
>>> getPendingReferences() native method. It turns out that a single native 
>>> method would not be enough to support the precise direct ByteBuffer 
>>> allocation. Here's a refactored webrev that introduces a 
>>> getPendingReferences() method which could be turned into a native 
>>> equivalent one day. There is another native method needed - int 
>>> awaitEnqueuePhaseStart():
>>> 
>>> 
>>> http://cr.openjdk.java.net/~plevart/jdk9-dev/removeInternalCleaner/webrev.09.part2/
>> I don't think the Reference.awaitEnqueuePhaseStart thing is needed.
>> 
>> Rather, I think the Direct-X-Buffer allocation should conspire with
>> the the Direct-X-Buffer cleanups directly to manage that sort of
>> thing, and not add anything to Reference and the reference processing
>> thread.  E.g. the phase and signal/wait are purely part of
>> Direct-X-Buffer.  (I also think something like that could/should have
>> been done instead of providing Direct-X-Buffer with access to
>> Reference.tryHandlePending, but that's likely water under the bridge
>> now.)
>> 
>> Something very roughly like this:
>> 
>> allocating thread, after allocation failed
>> 
>> bool waitForCleanups() {
>>   int epoch = DXB.getCleanupCounter();
>>   long start = startTime();
>>   long timeout = calcTimeout(start)
>>   synchronized (DXB.getCleanupMonitor()) {
>>     while (epoch == DBX.getCleanupCounter()) {
>>       wait(timeout);
>>       timeout = calcTimeout(start);
>>       if (timeout <= 0) break;
>>     }
>>     return epoch != DBX.getCleanupCounter();
>>   }
>> }
>> 
>> cleanup function, after freeing memory
>> 
>>   synchronized (DBX.getCleanupMonitor()) {
>>     DBX.incCleanupCounter();
>>     DBX.getCleanupMonitor().notify_all();
>>   }
>> 
>> Actually, epoch should probably have been obtained *before* the failed
>> allocation attempt, and should be an argument to waitForCleanups.
>> 
>> That's all quite sketchy, but I need to do other things today.
>> 
>> Peter, care to try filling this in?
>> 
>> 
> 
> There's no need to maintain a special cleanup counter as java.nio.Bits 
> already maintains the amount of currently allocated direct memory (in bytes). 
> What your suggestion leads to is similar to one of previous versions of 
> java.nio.Bits which waited for some 'timeout' time after invoking System.gc() 
> and then re-tried reservation, failing if it didn't succeed. The problem with 
> such "asynchronous" approach is that there's no right value of 'timeout' for 
> all situations. If you wait for to short time, you might get OOME although 
> there are plenty unreachable but still uncleaned direct buffers. If you wait 
> for to long, your throughput will suffer. There has to be some "feedback" 
> from reference processing to know when there's still beneficial to wait and 
> when there's no point in waiting any more.
> 
> Regards, Peter

I don't think there's any throughput penalty for a long timeout.  The
proper response to waitForCleanups returning false (assuming the epoch
was obtained early and passed as an argument) is OOME.  I really doubt
the latency for reporting OOME is of critical importance.

That is, the caller looks something like (not even pretending to write
Java) 

  alloc = tryAllocatation(allocSize)
  if alloc != NULL
    return alloc
  endif
  // Maybe add a retry+wait with a short timeout here,
  // to allow existing cleanups to run before requesting
  // another gc.  Not clear that's really worthwhile, as
  // it only comes up when we get here just after a gc
  // and the resulting cleanups are not yet all processed.
  System.gc()
  while true
    epoch = getEpoch() 
    alloc = tryAllocation(allocSize)
    if alloc != NULL
      return alloc
    elif !waitForCleanup(epoch)
      throw OOME  // No cleanup progress for a while
    endif
  end

Reply via email to