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

Tue Jan 12 15:19:25 UTC 2016

Alan and all,

Here’s the first version of this patch (along with a test):

http://cr.openjdk.java.net/~tonyp/nio-max-buffer-size/webrev.1/

A few points:

* I called the property java.nio.MaxCachedTempBufferSize. I’m happy to call it something else if you don’t like the name.

* I was not sure what the best way to read the property is. I took the same approach used for MaxDirectMemorySize and I read it from the saved properties on sun.misc.VM. Feel free to suggest a more appropriate alternative.

* I have compiled and run the test and it works as expected. But I haven’t run it within jtreg, though, given that I need to upgrade my jtreg to a more recent version. I’ll do that when I get a chance. But some tweaking on the metadata on the top comment might be required…

Regards,

Tony

On January 8, 2016 at 5:53:51 PM, Tony Printezis (tprintezis at twitter.com) wrote:

Alan,

I’m OK with using a property if that’s what’s more appropriate…

Yes, I agree that introducing a limit on the size of potentially cached buffers will be the simplest and will have fewer unexpected issues (like what you described). I can’t think of any other reasonable alternatives.

I’ll port the change to JDK 9 (our current release, and all our patches, are on JDK 8 right now) and I’ll post the diffs for feedback.

BTW, don’t know who’s the mod for the nio-dev list... any chance they can approve my previous message (and maybe this one)?

Regards,

Tony

On January 7, 2016 at 3:54:11 PM, Alan Bateman (alan.bateman at oracle.com) wrote:

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

Tony Printezis | JVM/GC Engineer / VM Team | Twitter

@TonyPrintezis
tprintezis at twitter.com

-----

Tony Printezis | JVM/GC Engineer / VM Team | Twitter

@TonyPrintezis
tprintezis at twitter.com

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/nio-dev/attachments/20160112/37c4f938/attachment-0001.html>