RFR (S): 8151322: Implement os::set_native_thread_name() on Solaris

[David Holmes]

Can I please get two re-reviews for this second version of the code. 
(I'm heading off on vacation mid-week and would like to get this in 
before I go.)

Thanks,
David

On 18/03/2016 7:02 AM, David Holmes wrote:
> On 17/03/2016 11:51 PM, Gerard Ziemski wrote:
>> David,
>>
>> If we were to move the initialization code off the startup path, we
>> wouldn’t need to bother with this optimization.
>>
>> I’m in favor of “defer until needed”, as I suggested earlier, as
>> opposed to the “eagerly initialize” approach taken here.
>
> That would make things worse. During startup we will call
> set_native_thread_name multiple times.
>
> David
>
>>
>> cheers
>>
>>> On Mar 16, 2016, at 8:43 PM, David Holmes <david.holmes at oracle.com>
>>> wrote:
>>>
>>> Hi James,
>>>
>>> On 17/03/2016 8:54 AM, james cheng wrote:
>>>> Hi David,
>>>>
>>>> Thanks for implementing this RFE.
>>>>
>>>> A Solaris Linker guru suggested what we do something like this:
>>>>
>>>>      my_pthread_setname_np(...)
>>>>      {
>>>>      static    (*fptr)() = NULL;
>>>>      void    *handle;
>>>>
>>>>      if ((fptr == NULL) &&
>>>>          ((handle = dlopen("libc.so.1", RTLD_LAZY)) != NULL) &&
>>>>          ((fptr = dlsym(handle, "pthread_setname_np")) == NULL))
>>>>          fptr = noop_setname;
>>>>
>>>>      (*fptr)(...);
>>>>      }
>>>>
>>>> So using libc.so.1 without a path should be okay.  But I don't see
>>>> you have
>>>> a null check for the return of dlsym().
>>>
>>> We initialize _pthread_setname_np using dlopen+dlsym as above at VM
>>> startup - we don't want to do the lookup on each use. The NULL check
>>> happens in the actual os::set_native_thread_name() function.
>>>
>>> Glad to hear using the libc.so.1 path is okay (as we already use it)
>>> but was wondering if we can assume /usr/lib/libc.so.1 and so shorten
>>> the search time even further?
>>>
>>> Thanks,
>>> David
>>>
>>>>
>>>> Thanks,
>>>> -James
>>>>
>>>> On 3/15/2016 9:24 PM, David Holmes wrote:
>>>>> cc'ing James as initial requestor for this.
>>>>>
>>>>> On 16/03/2016 7:49 AM, Gerard Ziemski wrote:
>>>>>>
>>>>>>> On Mar 15, 2016, at 4:31 PM, David Holmes <david.holmes at oracle.com>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> Couldn’t we look it up on as needed basis in the implementation of
>>>>>>>> "void os::set_native_thread_name(const char *name)” instead?
>>>>>>>
>>>>>>> AFAIK we never lookup anything as-needed but always handle it at VM
>>>>>>> initialization time. A quick grep will show that we are using
>>>>>>> RTLD_DEFAULT in a few places across
>>>>>>> different platforms. Elsewhere we know what library we have to
>>>>>>> search. I can try finding out which library it should be if you
>>>>>>> think that is preferable?
>>>>>>
>>>>>> Sure, either that or we find out the performance impact on the
>>>>>> startup time, so then we can decide if it’s an issue or not.
>>>>>
>>>>> Some numbers in the bug report. It seems to me if we know the library
>>>>> that will contain the symbol then we should just open it. I filed a
>>>>> RFE:
>>>>>
>>>>> https://bugs.openjdk.java.net/browse/JDK-8151953
>>>>>
>>>>> to look at use of RTLD_DEFAULT in general.
>>>>>
>>>>> Updated webrev: http://cr.openjdk.java.net/~dholmes/8151322/webrev.v2/
>>>>>
>>>>> Not 100% sure whether dlopen should be also relying on the search path
>>>>> dlopen("libc.so.1",...) - or whether the absolute /usr/lib/libc.so.1
>>>>> should be hard-wired? I'm not
>>>>> familiar enough with solaris library management to know whether we
>>>>> will always find libc on that exact path? We have one existing
>>>>> /usr/lib/libc.so.1 dlopen on Solaris x86,
>>>>> but most dlopens just take the base name.
>>>>>
>>>>> Thanks,
>>>>> David
>>>>> -----
>>>>>
>>>>>>
>>>>>> thanks
>>