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 <[email protected]
<mailto:[email protected]>> 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
<[email protected] <mailto:[email protected]>
<mailto:[email protected]
<mailto:[email protected]>>> 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
<[email protected] <mailto:[email protected]>
<mailto:[email protected]
<mailto:[email protected]>>> 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
