Hi David, builds and works on both variants (with and without USE_LIBRARY_BASED_TLS_ONLY) on AIX and Linux ppc.
Small nitpicks: - I probably would have implemented Thread::current() using Thread::current_or_null(). - Also, instead of using the "raw" ThreadLocalStorage::thread(), I would have liked a Thread::current_safe() or similar. Kind Regards, Thomas On Wed, Nov 11, 2015 at 9:23 PM, David Holmes <david.hol...@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.stu...@gmail.com >> <mailto:thomas.stu...@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.hol...@oracle.com <mailto:david.hol...@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 >> >> >> >>
