Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-06-22 Thread Peter Levart
On 06/22/2018 11:50 AM, Alan Bateman wrote: On 21/06/2018 18:29, Peter Levart wrote: On 06/21/2018 07:01 PM, Tony Printezis wrote: I’m trying exactly that. :-) Sorry, I didn't know. Here's my attempt: http://cr.openjdk.java.net/~plevart/jdk-dev/DBBCache_Cleanup/webrev.07/ I also added

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-06-22 Thread Peter Levart
Hi Tony, On 06/21/2018 07:39 PM, Tony Printezis wrote: Peter, The changes to TestMaxCachedBufferSize.java look fine. One point though: Why do you need the TmpDirectBuffersReclamation.java test? In TestMaxCachedBufferSize.java you just call checkDirectBuffers(0, 0); after you the main thread

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-06-22 Thread Alan Bateman
On 21/06/2018 18:29, Peter Levart wrote: On 06/21/2018 07:01 PM, Tony Printezis wrote: I’m trying exactly that. :-) Sorry, I didn't know. Here's my attempt: http://cr.openjdk.java.net/~plevart/jdk-dev/DBBCache_Cleanup/webrev.07/ I also added @run main/othervm to

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-06-21 Thread Tony Printezis
Peter, The changes to TestMaxCachedBufferSize.java look fine. One point though: Why do you need the TmpDirectBuffersReclamation.java test? In TestMaxCachedBufferSize.java you just call checkDirectBuffers(0, 0); after you the main thread calls join() on the workers? Tony — Tony Printezis |

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-06-21 Thread Peter Levart
On 06/21/2018 07:01 PM, Tony Printezis wrote: I’m trying exactly that. :-) Sorry, I didn't know. Here's my attempt: http://cr.openjdk.java.net/~plevart/jdk-dev/DBBCache_Cleanup/webrev.07/ I also added @run main/othervm to TempDirectBuffersReclamation test. Peter — Tony Printezis

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-06-21 Thread Tony Printezis
I’m trying exactly that. :-) — Tony Printezis | @TonyPrintezis | tprinte...@twitter.com On June 21, 2018 at 12:59:58 PM, Peter Levart (peter.lev...@gmail.com) wrote: On 06/21/2018 06:17 PM, Tony Printezis wrote: I was saying: I looked at TestMaxCachedBufferSize and, unfortunately, I

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-06-21 Thread Peter Levart
On 06/21/2018 06:17 PM, Tony Printezis wrote: I was saying: I looked at TestMaxCachedBufferSize and, unfortunately, I don’t think the test makes a lot of sense right now as it checks the number / size of direct buffers after all the threads terminate and, with this change, that should

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-06-21 Thread Tony Printezis
…and I also hadn’t attached the test. Sorry, I’m clearly very distracted today! — Tony Printezis | @TonyPrintezis | tprinte...@twitter.com On June 21, 2018 at 12:17:57 PM, Tony Printezis (tprinte...@twitter.com) wrote: (I unfortunately pressed Send accidentally; apologies) I was saying:

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-06-21 Thread Tony Printezis
(I unfortunately pressed Send accidentally; apologies) I was saying: I looked at TestMaxCachedBufferSize and, unfortunately, I don’t think the test makes a lot of sense right now as it checks the number / size of direct buffers after all the threads terminate and, with this change, that should

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-06-21 Thread Tony Printezis
Peter, Attached TerminatingThreadLocalTest.java. Let me know what you think (and feel free to modify it / discard it if you don’t like it!). Re: The test for the max cached buffer size is: test/jdk/sun/nio/ch/TestMaxCachedBufferSize.java. I looked at it and, unfortunately, — Tony Printezis

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-06-21 Thread Peter Levart
On 06/21/2018 05:53 PM, Peter Levart wrote: Hi, On 06/21/2018 09:42 AM, Alan Bateman wrote: On 20/06/2018 15:08, Peter Levart wrote: Like the following? http://cr.openjdk.java.net/~plevart/jdk-dev/DBBCache_Cleanup/webrev.05/ Yes, I think this looks good. Do we need a test which

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-06-21 Thread Peter Levart
Hi, On 06/21/2018 09:42 AM, Alan Bateman wrote: On 20/06/2018 15:08, Peter Levart wrote: Like the following? http://cr.openjdk.java.net/~plevart/jdk-dev/DBBCache_Cleanup/webrev.05/ Yes, I think this looks good. Do we need a test which proves that cached direct buffers are released

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-06-21 Thread Martin Buchholz
On Tue, Jun 19, 2018 at 6:06 AM, David Lloyd wrote: > It may be confusing (to some, I guess) but it is consistent: the > ThreadLocal subclass author has absolute control over how the value is > presented to the user. Adding compute() or many of the other > suggested variants will break that

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-06-21 Thread Peter Levart
On 06/20/2018 04:24 PM, Tony Printezis wrote: Hey Peter, I had written a test to test my version of the exit hooks. I can easily rework it to work with yours. Interested? Of course. Just give what you got. I can modify it as needed. To check that the native buffers are reclaimed

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-06-21 Thread Alan Bateman
On 20/06/2018 15:08, Peter Levart wrote: Like the following? http://cr.openjdk.java.net/~plevart/jdk-dev/DBBCache_Cleanup/webrev.05/ Yes, I think this looks good. Do we need a test which proves that cached direct buffers are released when thread exits or would a unit test for

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-06-20 Thread Tony Printezis
Hey Peter, I had written a test to test my version of the exit hooks. I can easily rework it to work with yours. Interested? To check that the native buffers are reclaimed promptly, we should be able to amend the test that tests the setting of the max size for the cached buffers (i.e., check

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-06-20 Thread Peter Levart
On 06/18/2018 05:41 PM, Alan Bateman wrote: On 17/06/2018 22:20, Peter Levart wrote: Update: the discussion on concurrent-interest about possible future addition of public ThreadLocal.getIfPresent() or ThreadLocal.isPresent() has died out, but it nevertheless reached a point where

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-06-19 Thread Tony Printezis
Good idea, as long as it re-uses the existing ThreadLocal infrastructure and doesn’t introduce an extra per-thread map. Making it a ThreadLocal subclass would be an excellent start. Tony — Tony Printezis | @TonyPrintezis | tprinte...@twitter.com On June 19, 2018 at 9:07:07 AM, David Lloyd

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-06-19 Thread David Lloyd
It may be confusing (to some, I guess) but it is consistent: the ThreadLocal subclass author has absolute control over how the value is presented to the user. Adding compute() or many of the other suggested variants will break that guarantee, which seems like kind of a big deal to me. What about

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-06-18 Thread Martin Buchholz
yes, the proposed new methods would use nulls differently. The original get() behavior of creating a mapping was a mistake, inconsistent with Map. Yes, it will cause confusion. But it's already confusing. On Mon, Jun 18, 2018 at 1:45 PM, David Lloyd wrote: > On Mon, Jun 18, 2018 at 12:53 PM,

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-06-18 Thread David Lloyd
On Mon, Jun 18, 2018 at 12:53 PM, Martin Buchholz wrote: > On Mon, Jun 18, 2018 at 10:21 AM, Tony Printezis > wrote: > >> Martin, >> >> You are forgiven. :-) >> >> So, the (valid, I think) issue with getIfPresent(), as originally proposed >> by Peter, was that if get() was overridden in the

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-06-18 Thread Martin Buchholz
On Mon, Jun 18, 2018 at 10:21 AM, Tony Printezis wrote: > Martin, > > You are forgiven. :-) > > So, the (valid, I think) issue with getIfPresent(), as originally proposed > by Peter, was that if get() was overridden in the subclass to somehow > transform the returned value, getIfPresent()

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-06-18 Thread Tony Printezis
Martin, You are forgiven. :-) So, the (valid, I think) issue with getIfPresent(), as originally proposed by Peter, was that if get() was overridden in the subclass to somehow transform the returned value, getIfPresent() wouldn’t apply the same transformation. Doesn’t your compute() method have

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-06-18 Thread Alan Bateman
On 17/06/2018 22:20, Peter Levart wrote: Update: the discussion on concurrent-interest about possible future addition of public ThreadLocal.getIfPresent() or ThreadLocal.isPresent() has died out, but it nevertheless reached a point where ThreadLocal.isPresent() was found the least

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-06-18 Thread Martin Buchholz
I'm ignoring the direct buffers problem (sorry Tony) and wearing my ReentrantReadWriteLock hat. I still like the idea of adding ThreadLocal.compute(Function remappingFunction) and perhaps other such map-inspired methods. RRWL wouldn't benefit much, since it already tries to minimize use of

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-06-17 Thread Peter Levart
Update: the discussion on concurrent-interest about possible future addition of public ThreadLocal.getIfPresent() or ThreadLocal.isPresent() has died out, but it nevertheless reached a point where ThreadLocal.isPresent() was found the least problematic. So I would like to get further with this

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-06-07 Thread Tony Printezis
Peter, Inline. — Tony Printezis | @TonyPrintezis | tprinte...@twitter.com On June 7, 2018 at 5:21:43 AM, Peter Levart (peter.lev...@gmail.com) wrote: Hi Tony, Thanks for taking a look. Just a couple of comments inline... On 06/06/18 22:38, Tony Printezis wrote: - instead of

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-06-07 Thread Peter Levart
Hi Tony, Thanks for taking a look. Just a couple of comments inline... On 06/06/18 22:38, Tony Printezis wrote: - instead of overriding initialValue(), ThreadLocal.setInitialValue() calls TerminatingThreadLocal.register() at the right moment Thanks. I much prefer not having to introduce

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-06-06 Thread Tony Printezis
Hi Peter, Thanks for the updated webrev! Please see inline. — Tony Printezis | @TonyPrintezis | tprinte...@twitter.com On June 6, 2018 at 2:55:51 PM, Peter Levart (peter.lev...@gmail.com) wrote: Ok, here's next webrev:

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-06-06 Thread Peter Levart
Ok, here's next webrev: http://cr.openjdk.java.net/~plevart/jdk-dev/DBBCache_Cleanup/webrev.03/ The changes from webrev.02 are: - renamed JdkThreadLocal -> TerminatingThreadLocal - instead of overriding initialValue(), ThreadLocal.setInitialValue() calls TerminatingThreadLocal.register() at

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-06-06 Thread Peter Levart
On 06/06/18 17:41, Tony Printezis wrote: Peter, You’re totally right re: JdkThreadLocal::initialValue being final and cannot be overridden (I had completely missed the final keyword when I first looked at the webrev). But it’d be nice to have the same API in both versions of ThreadLocal.

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-06-06 Thread Tony Printezis
Peter, You’re totally right re: JdkThreadLocal::initialValue being final and cannot be overridden (I had completely missed the final keyword when I first looked at the webrev). But it’d be nice to have the same API in both versions of ThreadLocal. I assume you didn’t want to override get() since

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-06-06 Thread Peter Levart
Hi Tony, Alan, On 06/06/2018 04:37 PM, Tony Printezis wrote: Alan, Thanks for your thoughts! Peter, Any chance of taking the two suggestions I made in an earlier e-mail into account? Right, I'll prepare new webrev with that shortly. a) It’d be nice if users can override initialValue(),

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-06-06 Thread Tony Printezis
Alan, Thanks for your thoughts! Peter, Any chance of taking the two suggestions I made in an earlier e-mail into account? a) It’d be nice if users can override initialValue(), like when using the standard ThreadLocal class, instead of calculateInitialValue(). (I can’t come up with a clean

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-06-06 Thread Alan Bateman
On 30/05/2018 22:16, Peter Levart wrote: I thought there would be some hint from Alan about which of the two paths we should take for more refinement. The Tony's: http://cr.openjdk.java.net/~tonyp/8202788/webrev.1/ Or the Alan's with my mods:

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-06-05 Thread Tony Printezis
Hey Alan, Any thoughts on this? (with apologies for the ping) Tony — Tony Printezis | @TonyPrintezis | tprinte...@twitter.com On May 30, 2018 at 5:16:44 PM, Peter Levart (peter.lev...@gmail.com) wrote: I thought there would be some hint from Alan about which of the two paths we should

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-05-30 Thread Peter Levart
I thought there would be some hint from Alan about which of the two paths we should take for more refinement. The Tony's: http://cr.openjdk.java.net/~tonyp/8202788/webrev.1/ Or the Alan's with my mods: http://cr.openjdk.java.net/~plevart/jdk-dev/DBBCache_Cleanup/webrev.02/ Regards, Peter

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-05-30 Thread Tony Printezis
Hi all, Any more thoughts on this? (with apologies for the ping) Tony — Tony Printezis | @TonyPrintezis | tprinte...@twitter.com On May 18, 2018 at 3:58:57 PM, Tony Printezis (tprinte...@twitter.com) wrote: Hi again, Stylistically, I strongly prefer this version over the previous one

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-05-18 Thread Tony Printezis
Hi again, Stylistically, I strongly prefer this version over the previous one (the one with the doubly-linked list of JdkThreadLocal entries) you had posted. This one is a lot cleaner. A few suggestions: * I’m not crazy about the fact that the users have to override calculateInitialValue()

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-05-18 Thread Tony Printezis
Hi Peter, Thanks for taking a look at the new webrev! Initially, I think we’re expecting two uses of JdkThreadLocal: one in sun.nio.ch.Utils, shown on my webrev and my original motivation for working on this, and one in sun.nio.fs.NativeBuffers, as shown on Alan’s webrev (I’m not familiar with

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-05-18 Thread Peter Levart
It's good to have alternative implementations on the table. Here's another variant that is space neutral for threads that don't use JdkThreadLocal, but also scales to many JdkThreadLocal instances: http://cr.openjdk.java.net/~plevart/jdk-dev/DBBCache_Cleanup/webrev.02/ Now we only need an

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-05-17 Thread Peter Levart
Hi Tony, If we anticipate only small number of JdkThreadLocal(s) (there will be only one for the start) then this is a plausible solution. Then perhaps this limitation should be written somewhere in the JdkThreadLocal javadoc so that one day somebody will not be tempted to use

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-05-17 Thread Tony Printezis
Hi all, I have a new version of the code for your consideration: http://cr.openjdk.java.net/~tonyp/8202788/webrev.1/ I basically combined our two approaches. The usage is as Alan had proposed it: Users have to use JdkThreadLocal (which is only available within java.base) and override

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-05-14 Thread Tony Printezis
Peter, In my proposal, you can register the exit hook in the ThreadLocal c’tor, so it’s almost as nice as Alan’s in that respect (and it doesn't require an extra field per ThreadLocal plus two extra fields per JdkEntry). :-) But, I do like the addition of the JdkEntry list to avoid unnecessarily

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-05-14 Thread Tony Printezis
Alan and Peter, First, I need to apologize: I completely missed Peter’s proposal (for some reason Peter’s follow-up e-mails show up blank on my mail client). Could someone point me to it so I can take a look? Re: having info per thread vs. globally: Having a couple more objects and a field

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-05-12 Thread Peter Levart
Hi, On 05/11/18 16:13, Alan Bateman wrote: On 08/05/2018 16:07, Tony Printezis wrote: Hi all, Following the discussion on this a few weeks ago, here’s the first version of the change: http://cr.openjdk.java.net/~tonyp/8202788/webrev.0/ I think the consensus was that it’d be easier if the

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-05-11 Thread Alan Bateman
On 08/05/2018 16:07, Tony Printezis wrote: Hi all, Following the discussion on this a few weeks ago, here’s the first version of the change: http://cr.openjdk.java.net/~tonyp/8202788/webrev.0/ I think the consensus was that it’d be easier if the exit hooks were only available within

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-05-08 Thread Tony Printezis
David, Please see inline. — Tony Printezis | @TonyPrintezis | tprinte...@twitter.com On May 8, 2018 at 11:44:11 AM, David Lloyd (david.ll...@redhat.com) wrote: I'm not a reviewer, but I would ask: how sure are we that it's OK to use lambdas from here? Is there a chance that the magic

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-05-08 Thread Alan Bateman
On 08/05/2018 16:43, David Lloyd wrote: : Also, I would be quite surprised if there wasn't a way to get a system property from system code without having to use doPrivileged; that might bear some researching. GetPropertyAction.privilegedGetProperty - there's an example usage in the

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

2018-05-08 Thread David Lloyd
I'm not a reviewer, but I would ask: how sure are we that it's OK to use lambdas from here? Is there a chance that the magic bootstrap stuff won't yet be initialized at the time when an early thread could exit for whatever reason? Anyway I think using a lambda with forEach is probably overkill