(L) Prelim RFR: 8132510: Replace ThreadLocalStorage with compiler/language-based thread-local variables
thomas.stuefe at gmail.com
Wed Nov 4 08:17:29 UTC 2015
On Wed, Nov 4, 2015 at 5:08 AM, David Holmes <david.holmes at oracle.com>
> Hi Thomas,
>> One question about your changes:
>> Before, Thread::current() would assert instead of returning NULL if
>> called before Thread::initialize_thread_current() or after
>> Thead::clear_thread_current() . Now, we just return NULL. Is this
> Ah great question ... so before we have a mix of calls to:
> - Thread::current() (asserts on NULL as does JavaThread::current)
> - ThreadLocalStorage::thread() (can return NULL)
> - ThreadLocalStorage::get_thread_slow() (can return NULL)
> and now we only have Thread::current() which means we have to allow
> returning NULL because it can be intentionally called when a thread is not
> attached. That means we won't directly catch calls to Thread::current()
> from code that doesn't realize it is calling it "too soon" - though there
> do exist numerous assertions in the callers of Thread::current() that check
> the result is not NULL.
> I could add the assert to Thread::current() and also add
> Thread::current_or_null() to be used by code that needs to use it to check
> for attachment (ie JNI code). I'd also have to examine all the changed
> ThreadLocalStorage::thread/get_thread_slow call-sites to see if any of
> those legitimately expect the thread may not be attached.
> What do you think?
I would prefer having Thread::current() to assert and to have a
Thread::current_or_null() for cases where NULL could occurr. I tend to hit
that assert a lot in development, it is useful. And the non-asserting
version gets already used in a number of places, also in our (not OpenJDK)
> I also need to look at the location of Thread::current in the .hpp file
> rather than .inline.hpp and reconcile that with comments regarding the
> noinline version (which is only used in g1HotCardCache.hpp).
Could we leave just the inline version in thread.hpp and remove the
noinline version altogether? Now that Thread::current() is very simple, we
may just as well keep it in the class body like the other accessors.
More information about the hotspot-dev