Request for review (M): 7132070: Use a mach_port_t as the OSThread thread_id rather than pthread_t on BSD/OSX

[Nils Loodin]

Does that not count under your mail about premature optimizations then?

:D

/Nisse

On Feb 15, 2012, at 21:46 , Daniel D. Daugherty wrote:

> On 2/15/12 1:34 PM, Staffan Larsen wrote:
>> That's what I guessed. So essentially all the code the is in the #else part of #ifdef _ALLBSD_SOURCE under src/os/bsd can be removed and the #ifdef _ALLBSD_SOURCE can be skipped. Like so:
>> 
>> #ifdef _ALLBSD_SOURCE
>>    xxx;
>> #else
>>    yyy;
>> #endif
>> 
>> can be changed to:
>> 
>> xxx;
>> 
>> I'll volunteer to do that cleanup.
> 
> I think that would make the eventual merge of all the "nixes"
> more difficult...
> 
> Dan
> 
> 
>> /Staffan
>> 
>> On 15 feb 2012, at 21:24, Daniel D. Daugherty wrote:
>> 
>>> The src/os/bsd code was derived from the src/os/linux code.
>>> #ifdef _ALLBSD_SOURCE was used to flag (most) changes relative
>>> to Linux for the BSD port. The #ifdef'ing is not perfect because
>>> the Linux code also evolved as the BSD port was being done and
>>> also because sometimes folks forgot to #ifdef their changes...
>>> 
>>> Eventually, there is supposed to be a rework/rewrite in this area
>>> where all the "nixes" share code. It won't be under src/os/posix,
>>> but we'll think of some good name if the project ever gets done...
>>> 
>>> Dan
>>> 
>>> 
>>> On 2/15/12 12:30 PM, Staffan Larsen wrote:
>>>> Reading this again, I wonder: What is the purpose of _ALLBSD_SOURCE in a file under 'src/os/bsd'? Are there platforms that are 'bad', but do not define _ALLBSD_SOURCE?
>>>> 
>>>> /Staffan
>>>> 
>>>> 
>>>> On 15 feb 2012, at 16:16, Daniel D. Daugherty wrote:
>>>> 
>>>>> The _ALLBSD_SOURCE symbol is defined by the HotSpot Makefile infrastructure.
>>>>> It is used to identify code specific to the BSD family of OSes.
>>>>> The __APPLE__ symbol is defined by the Apple compiler(s) and it is used to
>>>>> identify code specific to MacOS X.
>>>>> 
>>>>> Typically you'll see something like:
>>>>> 
>>>>> #ifdef _ALLBSD_SOURCE
>>>>> 
>>>>> <code that works on all BSDs>
>>>>> 
>>>>> #ifdef __APPLE__
>>>>> <code specific to MacOS X>
>>>>> #else
>>>>> <code for other BSDs>
>>>>> #endif // __APPLE__
>>>>> #endif // _ALLBSD_SOURCE
>>>>> 
>>>>> As for building on non-MacOS X BSDs, that would be nice, but we
>>>>> don't have the infrastructure to do it.
>>>>> 
>>>>> Dan
>>>>> 
>>>>> On 2/15/12 6:57 AM, Mikael Gerdin wrote:
>>>>>> Hi Staffan,
>>>>>> 
>>>>>> It looks like you're adding Mac-specific stuff like thread_t and calls to ::mach_thread_self() inside _ALLBSD_SOURCE #ifdefs, are you sure this won't break BSD builds?
>>>>>> Does the OSX compiler define _ALLBSD_SOURCE or is that for (free|net|open)bsd?
>>>>>> It's too bad we don't do regular builds on any of the BSDs, otherwise this would have been easier to figure out.
>>>>>> 
>>>>>> /Mikael
>>>>>> 
>>>>>> 
>>>>>> On 2012-02-15 11:29, Staffan Larsen wrote:
>>>>>>> Please review the following change:
>>>>>>> 
>>>>>>> Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7132070
>>>>>>> 
>>>>>>> Webrev: http://cr.openjdk.java.net/~sla/7132070/webrev.00/
>>>>>>> 
>>>>>>> This changes the value returned by OSThread::thread_id() and
>>>>>>> os::current_thread_id() on macosx to return the mach thread_t instead of
>>>>>>> pthread_t. There is a separate method OSThread:pthread_id() that returns
>>>>>>> the pthread_t.
>>>>>>> 
>>>>>>> The reason for this change is both that JFR would like a 4 byte value
>>>>>>> for thread id, and that SA requires access to the thread_t.
>>>>>>> 
>>>>>>> Thanks,
>>>>>>> /Staffan