On 15 feb 2012, at 20:03, Paul Hohensee wrote: > On 2/15/12 1:58 PM, Staffan Larsen wrote: >> 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. > > globalDefinitions_gcc.hpp is where these sorts of #includes currently live, > so I'd go with present practice rather than doing something new. If > you want to put the include of mach.h in os_bsd.inline.hpp, then you > should do something similar with all the platform-dependent includes > in globalDefinitions_gcc.hpp.
I'd actually like to have it in osThread_bsd.hpp, but that isn't possible. I'll put in gloabalDefinitions_gcc.hpp. > >> >>> 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. > > Even so, if we ever need to make hotspot work on non-osx bsd platforms, we'll > at least have markers to guide the work. OK - I'll try to fix things up. /Staffan > > Paul > >> >> /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
