RFR (S) CR 6857566: (bf) DirectByteBuffer garbage creation can outpace reclamation

[Alan Bateman]

On 06/10/2013 23:56, Peter Levart wrote:
> :
>
> It's not so much about helping to achieve better throughput (as I 
> noted deallocating can not be effectively parallelized) but to 
> overcome the latency of waking-up the ReferenceHandler thread. Here's 
> my attempt at doing this:
>
> http://cr.openjdk.java.net/~plevart/jdk8-tl/DyrectBufferAlloc/webrev.01/
>
> This is much simplified from my 1st submission of similar strategy. I 
> tried to be as undisruptive to current logic of Reference processing 
> as possible, but of course you decide if this is still too risky for 
> inclusion into JDK8. Cleaner is unchanged - it processes it's thunk 
> synchronously and ReferenceHandler thread invokes it directly. 
> ReferenceHandler logic is the same - I just factored-out the content 
> of the loop into a private method to be able to call it from nio Bits 
> where the bulk of change lies.
>
> :
>
> So what do you think? Is this still too risky for JDK8?
>
I looked at the latest webrev and I think the approach looks good.

I should explain that I did look into this issue about 3-4 years ago and 
at the time I experimented with the allocating threads waiting until the 
reference handler had drained the pending list. I didn't think of doing 
the assist at the time, hence I was interested to see the # allocations 
where it helped.

On the patch then I agree with Aleksey that moving the static 
initializer makes it less obvious that the only change is registering 
the shared secret (it's not a big deal of course).

The back-off before retrying looks good, I just wonder if 1ms is too low 
to start with.

On the interrupt then I think it's okay to just set the interrupt status 
as you are doing.

I see you switched the tracking for the management interface to use 
AtomicLong. Are you looking to improve the concurrency or is there 
another reason?

A minor coding convention but the break before "else" and "finally" is 
inconsistent in these areas. Another consistency point is that maxsleeps 
is a constant and so should probably be in uppercase.

A related piece of work is the FileChannel map implementation where 
there is a gc + retry if mmap fails. This could be changed to have a 
similar back-off/retry.

On the test then the copyright date is 2001-2007 so I assume this was 
copied from somewhere :-)  I agree with Aleksey on the test duration, 
especially if you can provoke OOME in less than 10 or 20 seconds on some 
machines.

As regards whether this should go into JDK 8 then the updated proposal 
is significantly less risky that the original proposal that changed the 
implementation to use weak references.

That said, this is a 13 year old issue that hasn't come up very often 
(to my knowledge anyway, perhaps because those making heavy use of 
direct buffers are pooling buffers rather than allocating and 
unreferencing). In additional we are close to the end of JDK 8 (ZBB is 
in 2.5 weeks time) and technically we have been in ramp down (meaning 
P1-P3 only) since mid-July.

-Alan.