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

[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 essentially the same issue? Apart from
that, I personally like this proposal as I agree: one look-up is always
better than two.

Tony


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


On June 18, 2018 at 10:13:02 AM, Martin Buchholz (martinrb at google.com)
wrote:

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 ThreadLocal, but it would at least allow get() and remove()
to be coalesced on read-unlock.

RRWL never changes from one non-null value to another, it only switches
between absent and present.  I'd like to avoid the two lookups due to get
and remove.  Here's a prototype that does not yet help RRWL, but helps
callers switching between non-null values, and could probably be extended
via surgery to ThreadLocalMap:

    public T compute(
            java.util.function.Function<? super T,? extends T>
remappingFunction) {
        final Thread currentThread = Thread.currentThread();
        final ThreadLocalMap map = getMap(currentThread);
        final ThreadLocalMap.Entry e = (map == null)
            ? null
            : map.getEntry(this);
        final T oldValue = (e == null) ? null : (T)e.value;
        final T newValue = remappingFunction.apply(oldValue);
        if (newValue == null) {
            if (oldValue != null) {
                map.remove(this);
            }
        } else if (e != null) {
            e.value = newValue;
        } else if (map != null) {
            map.set(this, newValue);
        } else {
            createMap(currentThread, newValue);
        }
        return newValue;
    }



On Sun, Jun 17, 2018 at 2:20 PM, Peter Levart <peter.levart at gmail.com>
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?
>
> Regards, Peter
>
>
>
> On 06/06/18 20:55, Peter Levart wrote:
>
>