On 15 feb 2012, at 19:49, Paul Hohensee wrote: > Also, > > You can move the #include of mach.h into globalDefinitions_gcc.hpp, > in the #ifdef __APPLE__ section. That way, it won't cause a compile- > time error on non-osx platforms that don't have it.
I dislike adding more stuff to globalDefinitions since that gets pulled in all over the place. I'd prefer to have #includes only where they are needed. > The declaration of _thread_id, thread_id() and set_thread_id() in os_bsd.hpp > can be put under a #ifdef __APPLE__, vis., > > #ifdef _ALLBSD_SOURCE > #ifdef __APPLE__ > thread_t _thread_id; > #else > pthread_t _thread_id; > #endif > #endif > > The uses of mach_thread_self() in os_bsd.cpp can be similarly conditioned. > > If there's a thread_t defined by non-osx bsd implementations, then > you don't need predication in os_bsd.hpp, though you'd still need it > in os_bsd.cpp. I'm assuming there's no mach_thread_self() on non-osx > bsd platforms. I could do that, but the question is how much work we should put into not trying to break something that we can't test anyway. There's no way, even if I do this that I can verify that it works. /Staffan > > Thanks, > > Paul > > On 2/15/12 1:33 PM, Paul Hohensee wrote: >> Imo we should at least try to make non-osx bsd builds work, since >> the original code did work for non-osx builds. This change doesn't >> do that. >> >> In globalDefinitions_gcc.hpp, if you keep the lines >> >> #undef ELF_AC >> #undef EFL_ID >> >> then you don't have to change vm_version_x86.hpp. >> >> Paul >> >> On 2/15/12 10:16 AM, 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 >>>>
