Consider making TerminatingThreadLocal public
Hi all, It came to my attention that you introduced TerminatingThreadLocal in openjdk lately as an “internal class”: http://hg.openjdk.java.net/jdk/jdk/rev/106dc156ce6b <http://hg.openjdk.java.net/jdk/jdk/rev/106dc156ce6b> I would like to take the chance to ask if you ever considered to make this “non-internal” as I think it would super helpful for some use cases. To give you some more context let me give you an example of why we would love it in netty. So in netty we sometimes pool “direct memory” in ThreadLocals [1] and have to do some tricks to ensure this direct memory is really released once a Thread exit. Which I think is also exactly one use-case you have here. Sure we can do this with a WeakReference + ReferenceQueue , Cleaner or finalizer but I feel exposing TerminatingThreadLocal would be a lot more elegant. Another advantage would be that we can be sure that the cleanup action would be called in the Thread that holds the value of the ThreadLocal, which is not really possible with using WeakReference + ReferenceQueue , Cleaner or finalizer. What we currently doing in netty is basically have our own ThreadLocal [1] implementation and out own Thread sub-class [2] that will ensure we call some method on our TheadLocal that gives us the chance to do the cleanup by wrapping the Runnable[3] that is passed in. This works great but we can not always guarantee that our sub-class of Thread is used to access our ThreadLocal implementation, which means the cleanup action will not run in this case. Thanks, Norman [1] https://github.com/netty/netty/blob/netty-4.1.26.Final/common/src/main/java/... <https://github.com/netty/netty/blob/netty-4.1.26.Final/common/src/main/java/io/netty/util/concurrent/FastThreadLocal.java> [2] https://github.com/netty/netty/blob/netty-4.1.26.Final/common/src/main/java/... <https://github.com/netty/netty/blob/netty-4.1.26.Final/common/src/main/java/io/netty/util/concurrent/FastThreadLocalThread.java> [3] https://github.com/netty/netty/blob/netty-4.1.26.Final/common/src/main/java/... <https://github.com/netty/netty/blob/netty-4.1.26.Final/common/src/main/java/io/netty/util/concurrent/FastThreadLocalRunnable.java>
So I take this as a sign that there is no interest in this ? Bye Norman
On 10. Jul 2018, at 12:06, Norman Maurer <norman.maurer@googlemail.com> wrote:
Hi all,
It came to my attention that you introduced TerminatingThreadLocal in openjdk lately as an “internal class”:
http://hg.openjdk.java.net/jdk/jdk/rev/106dc156ce6b <http://hg.openjdk.java.net/jdk/jdk/rev/106dc156ce6b>
I would like to take the chance to ask if you ever considered to make this “non-internal” as I think it would super helpful for some use cases. To give you some more context let me give you an example of why we would love it in netty.
So in netty we sometimes pool “direct memory” in ThreadLocals [1] and have to do some tricks to ensure this direct memory is really released once a Thread exit. Which I think is also exactly one use-case you have here. Sure we can do this with a WeakReference + ReferenceQueue , Cleaner or finalizer but I feel exposing TerminatingThreadLocal would be a lot more elegant. Another advantage would be that we can be sure that the cleanup action would be called in the Thread that holds the value of the ThreadLocal, which is not really possible with using WeakReference + ReferenceQueue , Cleaner or finalizer.
What we currently doing in netty is basically have our own ThreadLocal [1] implementation and out own Thread sub-class [2] that will ensure we call some method on our TheadLocal that gives us the chance to do the cleanup by wrapping the Runnable[3] that is passed in. This works great but we can not always guarantee that our sub-class of Thread is used to access our ThreadLocal implementation, which means the cleanup action will not run in this case.
Thanks, Norman
[1] https://github.com/netty/netty/blob/netty-4.1.26.Final/common/src/main/java/... <https://github.com/netty/netty/blob/netty-4.1.26.Final/common/src/main/java/io/netty/util/concurrent/FastThreadLocal.java> [2] https://github.com/netty/netty/blob/netty-4.1.26.Final/common/src/main/java/... <https://github.com/netty/netty/blob/netty-4.1.26.Final/common/src/main/java/io/netty/util/concurrent/FastThreadLocalThread.java> [3] https://github.com/netty/netty/blob/netty-4.1.26.Final/common/src/main/java/... <https://github.com/netty/netty/blob/netty-4.1.26.Final/common/src/main/java/io/netty/util/concurrent/FastThreadLocalRunnable.java>
Hi Norman, On 26/07/18 09:35, Norman Maurer wrote:
So I take this as a sign that there is no interest in this ?
I'm not sure that is the right conclusion to draw. It may well be that those involved in making the original change are too busy to respond right now. However, there are a two other important considerations worth bearing in mind. Firstly, if you look back through the core-libs-dev archive you will find discussion that preceded introduction of this change -- primarily between Alan Bateman and Tony Printezis as I recall but I think others were involved. I believe that discussion included debate as to how this change ought to expose the new capability either as a restricted API (internal to JDK runtime) or as an external API (open to JDK client apps). Its clear what the conclusion was. Have you checked the archive? If not I would advise doing so and then coming up with counter-arguments in any argument you present. That might encourage those who were involved in the discussion to consider your suggestion. Secondly, I am afraid you are a bit late to the party with this post. I realise that it is a genuine suggestion for improvement and not simply a drive-by request for others to make your life easier. However, it is hard for developers to return to a fix that is done and dusted and meets the agreed remit because of late-expressed concerns. By that stage they will be busy chasing other important new tasks. regards, Andrew Dinn ----------- Senior Principal Software Engineer Red Hat UK Ltd Registered in England and Wales under Company Registration No. 03798903 Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander
On 26/07/2018 10:45, Andrew Dinn wrote:
: Firstly, if you look back through the core-libs-dev archive you will find discussion that preceded introduction of this change -- primarily between Alan Bateman and Tony Printezis as I recall but I think others were involved. I believe that discussion included debate as to how this change ought to expose the new capability either as a restricted API (internal to JDK runtime) or as an external API (open to JDK client apps). Its clear what the conclusion was.
Thanks Andrew. Yes, there was lengthy discussion here, mostly Tony, Peter Levart and I. We decided not to expose anything at this time, an approach that does not preclude re-visiting it in the future. In general it would need a lot of consideration, esp. when you think about cases where a thread local maintains an object with a reference to a native resource and the object is reachable by other means. We also need to think about what it would mean further down the road if/when the platform is retrofitted to support fibers as there will have implications for what we know today as thread locals. -Alan
participants (3)
-
Alan Bateman
-
Andrew Dinn
-
Norman Maurer