RFR: 8132510: Replace ThreadLocalStorage with compiler/language-based thread-local variables
David Holmes
david.holmes at oracle.com
Tue Dec 1 21:54:55 UTC 2015
Thanks Thomas. I'll be trying to push this today.
David
On 1/12/2015 7:41 PM, Thomas Stüfe wrote:
> Hi David,
>
> On Thu, Nov 26, 2015 at 12:31 AM, David Holmes <david.holmes at oracle.com
> <mailto:david.holmes at oracle.com>> wrote:
>
> Hi Thomas,
>
> Thanks for reviewing.
>
> On 26/11/2015 1:20 AM, Thomas Stüfe wrote:
>
> 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.
>
>
> Thanks. Please let me know if any issues surface.
>
>
> Sorry for the delay. AIX tests went through without any issues related
> to your change. Our machines are terribly slow, so all I could do was
> hotspot/runtime though.
>
> But even if issues surface, we can easily backpaddle later to pthread tls.
>
> Kind Regards, Thomas
>
> I will need to resubmit my own tests after latest changes.
>
> 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.
>
>
> It's possible to file a RFE for this. To be honest I didn't see this
> as some kind of user-defined choice that would need to be
> configured. The intent is that platforms which can't support
> compiler level TLS simply hardwire the flag in their platform
> specific makefile.
>
> ------------
>
> 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();
>
>
> Indeed it can. Fixed. And changed jt to thread in the following code.
>
> ------------
>
> 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.
>
>
> Fixed.
>
> ----
>
> JavaThread* JavaThread::active() {
> - Thread* thread = ThreadLocalStorage::thread();
> + Thread* thread = Thread::current();
> assert(thread != NULL, "just checking");
>
> the assert is unnecessary.
>
>
> Fixed.
>
>
> -----
>
> 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.
>
>
> Fixed
>
> -----
>
> 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
>
>
> Okay - changed.
>
> Updated webrev: http://cr.openjdk.java.net/~dholmes/8132510/webrev.v9/
>
> Thanks,
> David
> -----
>
> -----
>
> Kind Regards, Thomas
>
>
>
>
>
>
>
>
> On Mon, Nov 23, 2015 at 8:03 AM, David Holmes
> <david.holmes at oracle.com <mailto:david.holmes at oracle.com>
> <mailto:david.holmes at oracle.com
> <mailto: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
>
>
>
More information about the hotspot-dev
mailing list