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

[Tony Printezis]

…and I also hadn’t attached the test. Sorry, I’m clearly very distracted
today!


—————
Tony Printezis | @TonyPrintezis | tprintezis at twitter.com


On June 21, 2018 at 12:17:57 PM, Tony Printezis (tprintezis at twitter.com)
wrote:

(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 always be 0.

Let me look into this and I’ll get back to you in a bit.

Tony


—————
Tony Printezis | @TonyPrintezis | tprintezis at twitter.com


On June 21, 2018 at 12:14:24 PM, Tony Printezis (tprintezis at twitter.com)
wrote:

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 | @TonyPrintezis | tprintezis at twitter.com


On June 21, 2018 at 7:07:59 AM, Peter Levart (peter.levart at gmail.com) wrote:



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 promptly, we should be able
to amend the test that tests the setting of the max size for the cached
buffers (i.e., check that, after all the threads have exited, the total
native count / size is 0).


Good idea. Do you happen to know which one is it?


Tony


—————
Tony Printezis | @TonyPrintezis | tprintezis at twitter.com


On June 20, 2018 at 10:08:48 AM, Peter Levart (peter.levart at gmail.com)
wrote:


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 ThreadLocal.isPresent() was found the least problematic.
>> So I would like to get further with this proposal using the last
>> webrev.04 version of the patch that uses ThreadLocal.isPresent()
>> internally - still package-private at the moment.
>>
>> Here's the webrev (unchanged from the day it was published):
>>
>> http://cr.openjdk.java.net/~plevart/jdk-dev/DBBCache_Cleanup/webrev.04/
>>
>> Would this version be OK?
> I think looks quite good.
>
> One small nit is that the update to ThreadLocal.setInitialValue makes
> it look like all locals are registered when setting the initial value.
> What would you think about moving the instanceof check from
> TerminatingThreadLocal.register so that it's a bit more obvious.
>
> -Alan

Like the following?

http://cr.openjdk.java.net/~plevart/jdk-dev/DBBCache_Cleanup/webrev.05/


Do we need a test which proves that cached direct buffers are released
when thread exits or would a unit test for TerminatingThreadLocal be
enough? Maybe both?

Regards, Peter