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

[David Holmes]

On 12/11/2015 11:51 PM, Thomas Stüfe wrote:
> Hi David,
>
> builds and works on both variants (with and without
> USE_LIBRARY_BASED_TLS_ONLY) on AIX and Linux ppc.

Great - thanks!

> Small nitpicks:
>
> - I probably would have implemented Thread::current() using
> Thread::current_or_null().

I can do that. I presume the compiler will be smart enough. :)

> - Also, instead of using the "raw" ThreadLocalStorage::thread(), I would
> have liked a Thread::current_safe() or similar.

That's a reasonable suggestion too - I was influenced by existing usage, 
but could change it.

I'll send out the formal RFR once I have checked performance and done 
more testing.

Thanks,
David

> Kind Regards, Thomas
>
>
> On Wed, Nov 11, 2015 at 9:23 PM, David Holmes <david.holmes at oracle.com
> <mailto:david.holmes at oracle.com>> wrote:
>
>     Sorry Thomas the all important:
>
>     src/os/posix/vm/threadLocalStorage_posix.cpp
>
>     was missing from the webrev. Now adding.
>
>     David
>     -----
>
>     On 12/11/2015 2:47 AM, Thomas Stüfe wrote:
>
>
>         On Wed, Nov 11, 2015 at 5:36 PM, Thomas Stüfe
>         <thomas.stuefe at gmail.com <mailto:thomas.stuefe at gmail.com>
>         <mailto:thomas.stuefe at gmail.com
>         <mailto:thomas.stuefe at gmail.com>>> wrote:
>
>              Hi David,
>
>              I get build errors on all my platforms.
>
>              I think the change misses #include
>         "runtime/threadLocalStorage.hpp"
>              in a couple of places, at least thread.cpp and possible
>         also the
>              os_xx_yy.cpp files.
>
>
>         Sorry, I have to correct myself. It is a linker error, I do not
>         find the
>         implementations for the ThreadLocalStorage class methods anywhere. I
>         applied your patch atop a freshly synced hs-rt repo:
>
>         - .../hotspot $ hg log -l 3
>         changeset:   9317:3b23f69bc887 8132510__thread_davids_change
>         qbase qtip tip
>         user:        stuefe
>         date:        Wed Nov 11 16:12:14 2015 +0100
>         summary:     imported patch 8132510__thread_davids_change
>
>         changeset:   9316:f17e5edbe761 qparent
>         user:        tschatzl
>         date:        Tue Nov 10 11:07:15 2015 +0100
>         summary:     8140689: Skip last young-only gc if nothing to do
>         in the
>         mixed gc phase
>         Reviewed-by: mgerdin, drwhite
>
>         on AIX I get:
>
>         ld: 0711-317 ERROR: Undefined symbol: .ThreadLocalStorage::thread()
>         ld: 0711-317 ERROR: Undefined symbol:
>         .ThreadLocalStorage::is_initialized()
>         ld: 0711-317 ERROR: Undefined symbol:
>         .ThreadLocalStorage::set_thread(Thread*)
>         ld: 0711-317 ERROR: Undefined symbol: .ThreadLocalStorage::init()
>
>         Am I building wrong?
>
>         Regards, Thomas
>
>
>              Will take another look tomorrow.
>
>              Thanks, Thomas
>
>              On Wed, Nov 11, 2015 at 9:19 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:
>
>                  Hi Thomas,
>
>                  Okay here's the next revision:
>
>         http://cr.openjdk.java.net/~dholmes/8132510/webrev.v5/
>
>                  I've reinstated a basic ThreadLocalStorage class which
>         will only
>                  need two implementations: a POSIX one, and a Windows
>         one (still
>                  TBD). This class is always initialized and
>                  ThreadLocalStorage::thread() is used from the signal
>         handlers
>                  (as today).
>
>                  For platforms that don't have __thread support they can
>         define
>                  USE_LIBRARY_BASED_TLS_ONLY at build time to only use the
>                  ThreadLocalStorage implementation.
>
>                  Obviously still need to get some performance numbers.
>
>                  I'd appreciate it if you could retest AIX, though as all
>                  platforms currently use pthread_get/setspecific I'm
>         confident
>                  there will be no platform issues.
>
>                  Thanks,
>                  David
>
>
>
>