sun.nio.ch.Util: Don't cache an unlimited amount of memory

[Alan Bateman]

On 06/01/2016 22:26, Tony Printezis wrote:
> Alan and Evan,
>
> First, apologies I’m a bit late on this thread.
>
> As Evan mentioned, the fix I implemented was to introduce a parameter 
> that sets the limit for the size of buffers added to the cache and 
> never cache a buffer whose size is over this limit. I followed the way 
> the -XX:MaxDirectMemorySize=… cmd line arg was implemented and added a 
> -XX:MaxCachedTempBufferSize=… cmd line arg to HotSpot and pass that 
> value via a property to Java (that’s what’s done for 
> MaxDirectMemorySize). We could leave it as a property, as Alan 
> suggested (no changes to HotSpot); I have no strong opinion either 
> way. I just thought that it was best to be consistent with how 
> MaxDirectMemorySize is set.
>
> With MaxCachedTempBufferSize a user can implicitly set the overall 
> memory used up by cached buffers (it will also depend on how many 
> buffers per thread are cached, one in our case, and the number of 
> threads that do operations that need to cache buffers). An alternative 
> approach I considered was to introduce a parameter that sets a max for 
> the total memory taken up by all cached buffers. This will be a bit 
> more user-friendly (the user will know exactly the max overhead of 
> those buffers). However, it’d also require synchronization between 
> threads when replacing cached buffers (as we’d have to keep track of 
> the total size of all cached buffers across all threads) and I wanted 
> to avoid that. It also has a very bad pathological edge case: a thread 
> might cache a very large buffer that’s just under the total limit and 
> prevent all other threads from caching anything.
>
> Any other alternatives that we could consider here?
>
> Alan, what would you like me to do next? Should I maybe create a CR 
> and post the diffs I have so you guys can take a look?
>
On -XX vs. a system property then one thing to say is that this buffer 
cache is specific to NIO channels. So that will probably influence the 
property or VM option name and maybe it would better to not tie HotSpot 
to specific areas like this.

On the value then the simplest is to set the maximum size of a buffer to 
cache. Scatter/gather I/O is relatively rare so this means that most of 
the time then only one buffer will be cached per thread. I see your 
point that a global limit would be more user-friendly but it could 
always lead to unpredictable performance when the thread count is high 
and the global limit is reached. In that scenario it would degrade to a 
malloc + free per I/O operation for threads that are unlucky enough not 
to have a cached buffer.

As regards moving forward then a patch or webrev on cr.openjdk.java.net 
would be best. I assume you still have access to JIRA and can push 
webrevs to cr.

-Alan
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/nio-dev/attachments/20160107/cb4db85f/attachment.html>