[concurrency-interest] ThreadLocal.getIfPresent()

Peter Levart peter.levart at gmail.com
Thu Jun 7 04:02:15 EDT 2018

Hi James,

On 06/07/18 07:36, James Roper via Concurrency-interest wrote:
> On Thu, 7 Jun 2018 at 04:59, Nathan and Ila Reynolds via 
> Concurrency-interest <concurrency-interest at cs.oswego.edu 
> <mailto:concurrency-interest at cs.oswego.edu>> wrote:
>     I think isPresent() should be added no matter what is decided
>     about getIfPresent().  This will allow for "null" being present in
>     ThreadLocal and detecting it. For example, think of a situation
>     where if the value is not present, you want to do some expensive
>     operation. At the end of the operation, you decide that nothing
>     should be stored in ThreadLocal, but you want to cache this
>     decision and prevent the expensive operation from being done
>     again.  Without isPresent(), then some dummy Object has to be
>     stored in ThreadLocal.  I have never run into this situation while
>     using ThreadLocal, but it is a situation I bump into in other
>     class designs.
>     As for getIfPresent(), I thought it was a good addition to the
>     API.  I cannot think about a situation where I would have needed
>     it.  If I did, I probably figured out some workaround.
> Allowing for null to be considered a present value, such that 
> isPresent() returns true if you set a thread local to null, presents a 
> big problem for gitIfPresent(), since what can getIfPresent() return, 
> that will allow distinguishing between a present null, and a non 
> present value? If it returns T, then it will return null for both 
> present nulls, and no value. If it returns Optional<T>, then there's 
> no way for it to return a present null, since Optional cannot contain 
> null. You'd need Optional<Optional<T>> to correctly distinguish 
> between a present value, and a null.
> But, Optional also offers a good solution to this without introducing 
> isPresent. You can encode a present empty value into a thread local 
> today by using ThreadLocal<Optional<T>>, and setting it to 
> Optional.empty. This makes the programming logic much cleaner, as it 
> makes it clear that this thread local is not just storing cached 
> values, it's storing cached empty values too, making it much easier to 
> avoid programming mistakes when working with it. And of course, this 
> is the entire reason for introducing an Optional type in the first 
> place, it allows you to encode the presentness or lack thereof into 
> the type system, instead of having to add special purpose methods like 
> isPresent everywhere where you have a container that may need to 
> indicate whether a value is present or not.
> So I actually think that isPresent() isn't as useful, since for the 
> use cases you describe using ThreadLocal<Optional<T>> is a better 
> design in the first place, but Optional<T> getIfPresent() does have 
> value as a convenience for when null means is not present. In an ideal 
> world, we'd change get() to return Optional<T>, and then everything 
> would be completely explicit, there's never any nulls, but of course 
> we can't do that due to backwards compatibility.

The problem with getIfPresent() returning null for 
ThreadLocal<Optional<X>> is it violates the convention that methods 
returning Optional<X> must never return null. So Optional<X> is not a 
good choice for the T in ThreadLocal<T>.

> I also like Martin's ideas of introducing compute* methods similar to 
> what Map offers.

Those methods are most welcome in concurrent scenarios where they 
provide atomicity. In single-threaded scenarios they provide a kind of 
unification of Map vs. ConcurrentMap API which is good to be able to 
create common logic that works in both settings. ThreadLocal is not a 
Map and is never going to be a Map (as a subtype) and is always used in 
single-threaded scenarios, so compute* -like methods in ThreadLocal 
would just be an attempt to try to optimize a read-compute-write 
scenario so that it would involve a single hash-lookup operation. As 
compute- -like methods need lambdas and lambdas typically have to 
capture state, the optimization attempt might not be that effective.

But I agree that compute* (or maybe just compute(BinaryOperator)) is a 
viable alternative to getIfPresent() and/or isPresent() from purely 
functional standpoint. What I (and others) are trying to achieve is a 
way to "peek" into the state of ThreadLocal without actually triggering 
the initialization which might be heavy.

Adding compute to ThreadLocal would also provide another (the third) 
alternative way of associating a value with current thread:

1 - using get() in combination with initialValue()
2 - using set()
3 - using compute()

Is this good or does it complicate the API too much?

Regards, Peter

>     -Nathan
>     On 6/6/2018 12:17 PM, Peter Levart via Concurrency-interest wrote:
>>     On 06/06/18 19:28, David Lloyd wrote:
>>>     On Wed, Jun 6, 2018 at 12:09 PM, Peter Levart<peter.levart at gmail.com> <mailto:peter.levart at gmail.com>  wrote:
>>>>     Di David,
>>>>     On 06/06/18 18:48, David Lloyd wrote:
>>>>     On Wed, Jun 6, 2018 at 10:17 AM, Peter Levart via Concurrency-interest
>>>>     <concurrency-interest at cs.oswego.edu>
>>>>     <mailto:concurrency-interest at cs.oswego.edu>  wrote:
>>>>     Are you thinking about possible "inconsistency" between overridden get() and
>>>>     not overridden getIfPresent() ? Like for example:
>>>     Exactly.  You could possibly get around the issue by declaring it like this:
>>>     public class ThreadLocal<T> {
>>>          // ...
>>>          public static <T> T getIfPresent(ThreadLocal<T> tl) {
>>>              return Thread.currentThread().hasThreadLocalInternal(tl) ?
>>>     tl.get() : null;
>>>          }
>>>          // ...
>>>     }
>>     Or there could simply be the following method in ThreadLocal:
>>     /**
>>      * @return true if current thread has a value present for this
>>     thread-local variable; false if not.
>>      */
>>     public boolean isPresent()
>>     Would that be preferable?
>>     Peter
>>     _______________________________________________
>>     Concurrency-interest mailing list
>>     Concurrency-interest at cs.oswego.edu
>>     <mailto:Concurrency-interest at cs.oswego.edu>
>>     http://cs.oswego.edu/mailman/listinfo/concurrency-interest
>     -- 
>     -Nathan
>     _______________________________________________
>     Concurrency-interest mailing list
>     Concurrency-interest at cs.oswego.edu
>     <mailto:Concurrency-interest at cs.oswego.edu>
>     http://cs.oswego.edu/mailman/listinfo/concurrency-interest
> -- 
> *James Roper*
> /Senior Developer, Office of the CTO/
> Lightbend <https://www.lightbend.com/> – Build reactive apps!
> Twitter: @jroper <https://twitter.com/jroper>
> _______________________________________________
> Concurrency-interest mailing list
> Concurrency-interest at cs.oswego.edu
> http://cs.oswego.edu/mailman/listinfo/concurrency-interest

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://cs.oswego.edu/pipermail/concurrency-interest/attachments/20180607/3c82ce20/attachment-0001.html>

More information about the Concurrency-interest mailing list