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

[Thomas Stüfe]

Hi David,

On Wed, Nov 4, 2015 at 5:08 AM, David Holmes <david.holmes at oracle.com>
wrote:

> 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
>> intended?
>>
>
> 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)
coding.


> 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.

Thanks, Thomas



>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/porters-dev/attachments/20151104/c1ab9c33/attachment-0001.html>