Thread Locals (was Re: State of Loom)

Wed May 27 19:57:44 UTC 2020

I dispute categorizing the reason as laziness. It is most definitely not laziness on my part. I work with a (much too) large code base and a constantly changing crew of developers. Some of our code is over 20 years old. Most of the code that we are actively maintaining is over 15 years old. And we are adding complex new features all the time. The people fixing the bugs and implementing the new features are not the people who wrote the code originally. Most of the time they are not the last person to significantly modify the code. Our biggest maintenance burden is code comprehension; the time required for a developer to understand the part of the code they need to work on. Anything that reduces the effort required to understand the code is a huge win.

A couple of the things that help: Code that doesn't exist is the easiest to comprehend. Code that the developers already understand is not a big problem. Using standard Java SE classes wins on both of these metrics. For our purposes standard classes have no code; they just work. (*) In theory new members of our crew already understand standard Java SE classes. If we write our own class to do what we could do with a standard Java SE class, the developers have comprehend it every place it is used. They have to consider whether it is the cause of the problem they are trying to solve. Realistically our code is not as robust as the standard Java SE classes. We try, but it's just not. Anything we write is much more likely to cause problems than a standard class.

So whenever possible we use standard Java SE classes if they do what we need. And ThreadLocal solves our Thingy problem. I seriously doubt there was much written in the early 2000's that suggested our use of ThreadLocal was inappropriate. "Laziness" contains a value judgement that I think is inappropriate. For us it is just practical engineering.

That said, I would be more than happy to replace our use of ThreadLocals to address contention issues with a standard Java SE class. That still leaves the parameter passing use case which honestly I feel is more problematic.

Douglas

(*) Yes there can be problems in standard classes. Sometimes the JavaDoc is lacking and we have to read the code to understand what is going on. That's dangerous as there's no guarantee that a future release will keep the same implementation. And on a very few occasions we have found bugs in standard classes. We don't even consider that possibility until all other alternative have be absolutely rolled out. Effectively we never look at the Java SE source.

> On May 27, 2020, at 11:54 AM, Brian Goetz <brian.goetz at oracle.com> wrote:
> 
> There are two things being conflated regarding this use of TLs:
> 
>  - What is the right mechanism (hint: TL is probably not it);
>  - Who should provide that mechanism
> 
> I think Doug's main point here is that, if the JDK doesn't provide a better way, people will continue using TL out of laziness, with bad results.  
> 
> That's true, but I think we should focus on "what is the right mechanism" first.
> 
> On 5/27/2020 2:27 PM, Douglas Surber wrote:
>> Abstracting what I want from any hint of implementation, I'd like something like this:
>> 
>> /**
>>  * Maintains a pool of instances of T that are reused by multiple callers. Creates
>>  * a new instance of T only if all existing instances are in use at the time of a
>>  * call.
>>  */
>> public abstract class Pool<T> {
>> 
>> 	/**
>> 	 * Return a new instance of Pool that shares instances of U created
>> 	 * by creator. 
>> 	 */
>> 	public static <U> Pool<U> newInstance(Supplier<U> creator) { ... }
>> 
>> 	/**
>> 	 * Provides exclusive use of an instance of T for the duration of a call. Returns
>> 	 * the result of executing operation. It is erroneous for operation to capture
>> 	 * the instance of T. Supports multiple simultaneous calls without blocking.
>> 	 */
>> 	public abstract <V> call(Function<T, V> operation);
>> }
>> 
>> Using ThreadLocals I write
>> 
>> 	Foo f = thingyThreadLocal.get().getFoo(x);
>> 	// whatever
>> 	thingyThreadLocal.get().putFoo(F);
>> 
>> Using Pool I would write
>> 
>> 	Foo f = thingyPool.call(t -> t.getFoo(x));
>> 	// whatever
>> 	thingyPool.call(t -> t.putFoo(f)); 
>> 
>> I don't care if I get a Foo from one Thingy and put it back in another.
>> 
>> A Thingy would be checked out of the pool only for the duration of the call to Function.apply. My guess is this would minimize the number of instances of Thingy created while still eliminating all contention. 
>> 
>> Can I write this? Yes. Do I want to? No. Even more so I don't want to require my team to maintain it.
>> 
>> Douglas
>> 
>>> On May 27, 2020, at 11:11 AM, Mike Rettig <mike.rettig at gmail.com> <mailto:mike.rettig at gmail.com> wrote:
>>> 
>>> Keeping thread locals per OS carrier thread will lead to difficult to
>>> predict reordering bugs. Code will behave differently depending on
>>> whether it is executing on a virtual thread or a plain OS thread.
>>> 
>>> For example:
>>> 
>>> Virtual thread 1:
>>> MyConnection conn = threadLocal.get();
>>> conn.write("1");
>>> log.info("here!"); <-- BLOCKING so loom can park the virtual thread
>>> and reuse the carrier thread for a different virtual thread
>>> conn.write("2");
>>> 
>>> Virtual thread 2:
>>> Myconnection conn = threadLocal.get();
>>> conn.write("3");
>>> 
>>> This could result in the output of "132" or "312" or "123" for two
>>> virtual threads.
>>> 
>>> Mike
>>> On Wed, May 27, 2020 at 12:29 PM Remi Forax <forax at univ-mlv.fr> <mailto:forax at univ-mlv.fr> wrote:
>>>> I believe we need a way to say if a ThreadLocal means virtual thread locals or carrier thread local.
>>>> 
>>>> regards,
>>>> Rémi
>>>> 
>>>> [1] https://urldefense.com/v3/__https://github.com/openjdk/loom/blob/fibers/src/java.base/share/classes/java/lang/StringCoding.java*L66__;Iw!!GqivPVa7Brio!JsKGbvZS6owCm9ujLR5N3vgnywH1NeRo5VZXzMYL8VPvpMpMiZ4inVS2x1qAzp8_KAo$ <https://urldefense.com/v3/__https://github.com/openjdk/loom/blob/fibers/src/java.base/share/classes/java/lang/StringCoding.java*L66__;Iw!!GqivPVa7Brio!JsKGbvZS6owCm9ujLR5N3vgnywH1NeRo5VZXzMYL8VPvpMpMiZ4inVS2x1qAzp8_KAo$> 
>>>> 
>>>> ----- Mail original -----
>>>>> De: "Douglas Surber" <douglas.surber at oracle.com> <mailto:douglas.surber at oracle.com>
>>>>> À: "Andrew Haley" <aph at redhat.com> <mailto:aph at redhat.com>
>>>>> Cc: "loom-dev" <loom-dev at openjdk.java.net> <mailto:loom-dev at openjdk.java.net>
>>>>> Envoyé: Mercredi 27 Mai 2020 19:08:19
>>>>> Objet: Re: Thread Locals (was Re: State of Loom)
>>>>> Most of this code was written many, many years ago. We demonstrated that holding
>>>>> a Thingy in a static field resulted in excessive contention on the Thingy.
>>>>> Rather than invent a new data structure that we would have to maintain, we
>>>>> chose to leverage the tool that the Java class library provided, ie
>>>>> ThreadLocal. Could we do something different? Certainly. But having to roll our
>>>>> own increases our (already excessive) maintenance burden. I'd much rather use
>>>>> standard tools when possible, and in my use case ThreadLocal is acceptable. For
>>>>> now.
>>>>> 
>>>>> If the need arose to eliminate the ThreadLocals we would attempt to make the
>>>>> Thingys non-blocking. The benefit of that is eliminating the cost of having
>>>>> multiple Thingys. From experience making other parts of our code non-blocking I
>>>>> have reason to think that making these Thingys non-blocking would be
>>>>> challenging and would cause maintenance problems at least in the short term.
>>>>> These Thingys are stable so once we got all the bugs ironed out we'd be ok.
>>>>> Only if we couldn't make one or more non-blocking would be roll our own
>>>>> non-blocking Thingy sharing tool. Still, I'd much prefer to use a tool Java
>>>>> provides and I don't think I'm alone.
>>>>> 
>>>>> Douglas
>>>>> 
>>>>>> On May 27, 2020, at 9:16 AM, Andrew Haley <aph at redhat.com> <mailto:aph at redhat.com> wrote:
>>>>>> 
>>>>>> On 27/05/2020 16:03, Douglas Surber wrote:
>>>>>> 
>>>>>>> My code has multiple relatively expensive shared resources. Every
>>>>>>> call to my code potentially needs access to one or more of those
>>>>>>> shared resources for a brief moment. These resources are expensive
>>>>>>> to create and consume non-trivial heap space. Use of the resources
>>>>>>> is a simple call: call a method on the resource; get a
>>>>>>> response. Experience has shown that having one instance of each
>>>>>>> resource in a static field leads to unacceptable
>>>>>>> contention. Balancing cost of the resources with contention we keep
>>>>>>> the resources in ThreadLocals. Each user call can get use of a
>>>>>>> resources without contention while still limiting the total number
>>>>>>> of resources created.
>>>>>> Let's call your relatively expensive shared resource a Thingy, for
>>>>>> short.
>>>>>> 
>>>>>> ThreadLocals are not cheap: the last time I traced through it, there
>>>>>> were about 12 field loads and 5 conditional branches for a tl.get(),
>>>>>> best case. It can't be so hard to keep a global shared linked list of
>>>>>> Thingys and whenever you want one, remove it from the list with a
>>>>>> getAndSet() operation, and remember to put it back when you're
>>>>>> done. Is doing that really much more expensive than a
>>>>>> ThreadLocal.get() ? Or maybe the problem is that of lifetimes, in that
>>>>>> none of the existing code knows when it no longer needs its Thingy;
>>>>>> but that can't be true because you said that you only need them for a
>>>>>> brief moment.
>>>>>> 
>>>>>> --
>>>>>> Andrew Haley  (he/him)
>>>>>> Java Platform Lead Engineer
>>>>>> Red Hat UK Ltd.
>>>>>> <https://urldefense.com/v3/__https://www.redhat.com__;!!GqivPVa7Brio!NPUffY9EmeqSPKmerY_5RK8hZVg0Snysse_h8QxZXEHqSwEDd-HfD2yUBheAR8vnC48$ <https://urldefense.com/v3/__https://www.redhat.com__;!!GqivPVa7Brio!NPUffY9EmeqSPKmerY_5RK8hZVg0Snysse_h8QxZXEHqSwEDd-HfD2yUBheAR8vnC48$>
>>>>>> https://urldefense.com/v3/__https://keybase.io/andrewhaley__;!!GqivPVa7Brio!NPUffY9EmeqSPKmerY_5RK8hZVg0Snysse_h8QxZXEHqSwEDd-HfD2yUBheAxF1XcaU$ <https://urldefense.com/v3/__https://keybase.io/andrewhaley__;!!GqivPVa7Brio!NPUffY9EmeqSPKmerY_5RK8hZVg0Snysse_h8QxZXEHqSwEDd-HfD2yUBheAxF1XcaU$>
>>>>>> EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671
> 