(L) Prelim RFR: 8132510: Replace ThreadLocalStorage with compiler/language-based thread-local variables

[David Holmes]

Hi Thomas,

On 2/11/2015 9:20 PM, Thomas Stüfe wrote:
> Hi David,
>
> some small changes are needed to make this build and run on AIX. I
> attached a patch file with the needed additions.

Thanks!

> I did not run any extensive tests on AIX, so I cannot say for sure if
> this is stable. We (SAP) also may face some problems later when we port
> this to HP-UX, because there, shared libraries using __thread cannot be
> loaded dynamically.

Ouch!

> So, I admit to some small worries, beside the issue with memory leaks on
> older glibc versions. For me, this feels like something which needs
> tight compiler/thread library support from the OS, so it makes us
> vulnerable to running on older systems (older glibc) or building with
> outdated compilers. Therefore it would be nice to have a simple way to
> re-add the pthread-based TLS implementation if needed.

I can't see how to do that without keeping all the existing layers of 
code - even though they would be no-ops on all the platforms that 
support the compiler-based TLS. Basically just extend what I did for 
Solaris to the other platforms.

> Apart from that, I like the patch and think the simplification is good
> and worth the effort.

Even if you can't easily add back the pthread-based TLS if needed?

It is unfortunate that hotspot may still be shackled to the past that 
way - we killed off hotspot-express (in part) to remove those shackles 
and allow us to modernize the codebase.

Thanks,
David

> Kind Regards, Thomas
>
>
>
>
>
>
> On Mon, Nov 2, 2015 at 7:40 AM, David Holmes <david.holmes at oracle.com
> <mailto:david.holmes at oracle.com>> wrote:
>
>     bug: https://bugs.openjdk.java.net/browse/JDK-8132510
>
>     Open webrev: http://cr.openjdk.java.net/~dholmes/8132510/webrev.v2/
>
>     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 platform-specific ThreadLocalStorage
>     implementations, and the associated os::thread_local_storage* calls,
>     plus all the uses of ThreadLocalStorage::thread() and
>     ThreadLocalStorage::get_thread_slow(). This extends the previous
>     work done on Solaris to implement ThreadLocalStorage::thread() using
>     compiler-based thread-locals.
>
>     We can also consolidate nearly all the os_cpu versions of
>     MacroAssembler::get_thread on x86 into one cpu specific one ( a
>     special variant is still needed for 32-bit Windows).
>
>     As a result of this change we have further potential cleanups:
>     - all the src/os/<os>/vm/thread_<os>.inline.hpp files are now
>     completely empty and could also be removed
>     - the MINIMIZE_RAM_USAGE define (which avoids use of the linux
>     sp-map "cache" on 32-bit) now has no affect and so could be
>     completely removed from the build system
>
>     I plan to do the MINIMIZE_RAM_USAGE removal as a follow up CR, but
>     could add the removal of the "inline" files to this CR if people
>     think it worth removing them.
>
>     I have one missing piece on Aarch64 - I need to change
>     MacroAssembler::get_thread to simply call Thread::current() as on
>     other platforms, but I don't know how to write that. I would
>     appreciate it if someone could give me the right code for that.
>
>     I would also appreciate comments/testing by the AIX and PPC64 folk
>     as well.
>
>     A concern about memory-leaks had previously been raised, but
>     experiments using simple C code on linux 86 and Solaris showed no
>     issues. Also note that Aarch64 already uses this kind of thread-local.
>
>     Thanks,
>     David
>
>