RFR: 8230877: Rename THREAD_LOCAL_DECL to thread_local

[Per Liden]

On 9/12/19 2:04 AM, Kim Barrett wrote:
>> On Sep 11, 2019, at 7:46 PM, David Holmes <david.holmes at oracle.com> wrote:
>>
>> But as we are not actually enabling this as we don't turn on C++11, surely this still "looks good" enough? But we will have to look closer at things before turning it on? We may need to clarify our own rules for using thread-locals if we find there is a performance hit and we want to use the magic flag.
> 
> Part of the rationale for renaming to thread_local was that the
> transition to C++11 was trivial.  But apparently not so.
> 
> I really don't want to be tripping over this hidden performance hit
> when trying to throw the language standard switch.  (Assuming it's
> measurable, but we hit Thread::current() quite a bit, and ZGC has a
> bunch of thread-local variables pretty baked in and used fairly
> heavily I think.)  I do not want this little syntactic nicety to be an
> argument for not throwing that switch.
> 
> And in the long run, I'd prefer a different name if we're going to
> effectively have non-standard semantics (because of the usage
> restrictions required by that compiler flag).
> 

Ok, thanks for spotting the difference in gcc.

Given that they are said to be equivalent on windows and solaris, it 
sounds like we're already taking the dynamic init hit on those 
platforms. That's not necessarily a good reason to take that penalty on 
linux too, just an observation.

I looked closer at what the overhead is with gcc. For fundamental types 
and pointers (like our Thread::_thr_current), there's an extra 
test-and-branch sequence before the load.

	cmpq	$0, xxxx at GOTPCREL(%rip)
	je	done
	call	slow_path
done:

I've confirmed that -fno-extern-tls-init removes that test-and-branch.

I'm not super keep on spending too much time on investigating if this 
overhead is negligible or not. I also agree that messing with the 
thread_local semantics using -fno-extern-tls-init, or calling something 
thread_local that has non-standard semantics seems undesirable.

So, how about I dial this back a bit, and just rename THREAD_LOCAL_DECL 
to THREAD_LOCAL, just to make it a bit less clunky? That doesn't give us 
all the benefits I originally had in mind, but given the constraints we 
have here I don't see any better alternatives.

Updated webrev: http://cr.openjdk.java.net/~pliden/8230877/webrev.2

/Per