RFR: 8230877: Rename THREAD_LOCAL_DECL to thread_local
kim.barrett at oracle.com
Wed Sep 11 21:37:14 UTC 2019
> On Sep 11, 2019, at 4:37 PM, Per Liden <per.liden at oracle.com> wrote:
> ZGC uses thread locals in a number of places. It's currently using using the gcc specific __thread, but we'd like to make that compiler agnostic. We currently have the THREAD_LOCAL_DECL macro, which does what we want. However, I propose that we rename THREAD_LOCAL_DECL to thread_local, for the following reasons:
> * In C+11, thread_local was standardized. When we upgrade to C++1X, we can just remove our own thread_local macros, without the need to touch any other code.
> * The thread_local keyword is recognized by IDEs/editors, so gets correctly highlighted, etc.
> * THREAD_LOCAL_DECL looks a bit clunky in declarations (IMHO).
> About the patch:
> * USE_LIBRARY_BASED_TLS_ONLY now only controls how Thread::current() is implemented, not whether thread_local should be defined or not.
> * We define thread_local on gcc/clang/solstudio, if we're using a pre-C++11 compiler.
> * We don't define thread_local on xlc, since thread_local will never be used anyway because it currently always defines USE_LIBRARY_BASED_TLS_ONLY to 1.
> * We don't define thread_local on VS2017, since it's already supported with that compiler.
> Bug: https://bugs.openjdk.java.net/browse/JDK-8230877
> Webrev: http://cr.openjdk.java.net/~pliden/8230877/webrev.0
> Testing: Builds on all Oracle supported platforms
Sorry for misleading you in our earlier off-line discussion. I forgot about
older versions of VS.
For Visual Studio, while we (Oracle) no longer build jdk/jdk with anything
older than VS2017 (which unconditionally provides C++11 thread_local), other
people may still be building with older versions of VS. (That will have to
change when we start using C++11/14 features, but until then we've not been
pruning older versions from the set that's permitted, which includes some
pretty old versions.)
And checking __cplusplus (like we're doing on other platforms) seems to be a
bit of a mess for VS:
Instead for VS we probably need to do an MSVC version check to decide
whether to define thread_local to __declspec(thread), but I don't know which
version of VS starts providing it natively. Either that or prune the set of
(at least purportedly) supported versions somewhat.
More information about the hotspot-dev