RFR: 8132510: Replace ThreadLocalStorage with compiler/language-based thread-local variables

[Thomas Stüfe]

Hi David,

I looked over your latest version at
http://cr.openjdk.java.net/~dholmes/8132510/webrev.v8

This all looks nice and builds on AIX. I am currently running some jtreg
tests.

Some small remarks:

------------

would it be possible to add a configure option (e.g.
--enable-compiler-based-tls)? On AIX, disabling compiler level tls also
makes the -qtls compiler option unnecessary, so it affects the makefile.

Not really important, just would be quite comfortable.

------------

http://cr.openjdk.java.net/~dholmes/8132510/webrev.v8/src/os/windows/vm/os_windows.cpp.udiff.html

@@ -5175,11 +5159,11 @@

-  JavaThread* thread = (JavaThread*)(Thread::current());
+  Thread* thread = Thread::current();
   assert(thread->is_Java_thread(), "Must be JavaThread");
   JavaThread *jt = (JavaThread *)thread;

may be simplified to use your new

JavaThread* thread = JavaThread::current();

------------

http://cr.openjdk.java.net/~dholmes/8132510/webrev.v8/src/share/vm/runtime/thread.cpp.udiff.html

in

Thread::clear_thread_current()

we could also add a

assert(Thread::current() == ThreadLocalStorage::thread(), "TLS mismatch!");

to catch a case where library tls slot content changed during lifetime of
thread.

----

 JavaThread* JavaThread::active() {
-  Thread* thread = ThreadLocalStorage::thread();
+  Thread* thread = Thread::current();
   assert(thread != NULL, "just checking");

the assert is unnecessary.

-----

http://cr.openjdk.java.net/~dholmes/8132510/webrev.v8/src/share/vm/runtime/thread.hpp.udiff.html

 // Inline implementation of JavaThread::current
 inline JavaThread* JavaThread::current() {
-  Thread* thread = ThreadLocalStorage::thread();
+  Thread* thread = Thread::current();
   assert(thread != NULL && thread->is_Java_thread(), "just checking");
   return (JavaThread*)thread;
 }

asserting thread != NULL is unnecessary.

-----

http://cr.openjdk.java.net/~dholmes/8132510/webrev.v8/src/share/vm/utilities/globalDefinitions_gcc.hpp.udiff.html

(and corresponding globalDefinitions_.. files)

+#define THREAD_LOCAL_DECL __thread

could be surrounded by #ifndef USE_LIBRARY_BASED_TLS_ONLY

-----

Kind Regards, Thomas








On Mon, Nov 23, 2015 at 8:03 AM, David Holmes <david.holmes at oracle.com>
wrote:

> After all the preliminary discussions here are final proposed changes:
>
> bug: https://bugs.openjdk.java.net/browse/JDK-8132510
>
> Open webrev: http://cr.openjdk.java.net/~dholmes/8132510/webrev.v6/
>
> A simple (in principle) but wide-ranging change which should appeal to our
> Code Deletion Engineer's. We implement Thread::current() using a
> compiler/language-based thread-local variable eg:
>
>  static __thread Thread *_thr_current;
>
>  inline Thread* Thread::current() {
>    return _thr_current;
>  }
>
> with an appropriate setter of course. By doing this we can completely
> remove the os_cpu-specific ThreadLocalStorage implementations, and the
> associated os::thread_local_storage* calls.
>
> As __thread is not async-signal-safe (either in theory or practice) we
> need a fallback library-based solution as used today. For this we use a
> very simple ThreadLocalStorage class and an implementation thereof for
> POSIX (covers AIX, BSD, Linux, OSX and Solaris) using
> pthread_get/setspecific; and one for Windows using its TLS library. While
> these library routines are not guaranteed async-signal-safe, they seem to
> be in practice and are what we have been using all along.
>
> We also allow for use of only the library-based TLS for platforms where
> compiler-based thread locals are not supported (as will be needed in the
> Mobile project). This is enabled at build time by defining
> USE_LIBRARY_BASED_TLS_ONLY.
>
> Thanks to Andrew Haley for providing the Aarch64 code; and for Thomas
> Stuefe for testing PPC and AIX.
>
> Testing:
>  - JPRT (core platforms)
>  - Jtreg tests (linux & solaris)
>  - vm.runtime (core platforms)
>
> Performance:
>  - still TBD - this is proving to be extremely difficult. If anyone has
> any simple to run microbenchmarks to suggest I'd give them a try as a
> sanity check. But I lack access to hardware for running serious
> benchmarking.
>
> Footprint:
> - varies by platform and the VM (server, client, minimal)
> - worst-case: ~1% increase for server VM and minimal VM
> - best-case:  0.4% decrease for client VM
>
> Thanks,
> David
>