Hi Mark -
I'd prefer that it's done the "right" way (based on *my* definition of
"right", of course :) ), but I won't put up a fuss if whomever shepherds
this through agrees with you and wants to keep it in it's current form.
I expect that will be Serguei, or someone else from the serviceability
team? (Serguei?)
--
- Keith
On 5/23/2012 12:19 PM, Mark Wielaard wrote:
On Wed, 2012-05-23 at 10:23 -0400, Keith McGuigan wrote:
On 5/23/2012 9:56 AM, Mark Wielaard wrote:
It's (of course) just a style thing, but traditionally in hotspot
we've wanted the os or arch specific code in os or arch specific
directories, instead of littering the code with #ifdefs. I know the OSX
stuff started violated this some, but I hope we're going to resolve that
rather than continue down that path.
I wouldn't have mind if the OSX stuff was split out this way, but now I
don't have any example to follow here. What/How do you suggest it is
done?
My first thought would be to create a dtrace_solaris.hpp file in
src/os/solaris/vm and a dtrace_linux.hpp file in src/os/linux/vm (and
maybe the same for bsd and windows?) for any OS-specific stuff, like the
macro definitions and maybe even that solaris workaround. Leave
anything common in the original file (like the default empty definitions
and such). Then you include the specific-file based on the target OS,
similar to what's done here:
http://hg.openjdk.java.net/hsx/hotspot-main/hotspot/file/03d61caacd1e/src/share/vm/runtime/interfaceSupport.hpp
Yeah... it's a bit of a pain, and maybe in this case it's a little
overkill. But it's definitely nice to have a location for the
OS-specific stuff and it keeps the code rather neat.
So, do you require this change or not? I don't particularly like that
style (you loose the overview/a central place where all the magic macros
are defined), but I can certainly do it if you require it now. But then
I want to do it as additional patches on top of the current patches to
make sure they are separate. Since I will then have to touch code for at
least two architectures to which I don't have access (I also don't have
access to windows nor bsd, but I hope I don't have to touch those too).
So I want such changes to be testable separately by the architecture
maintainers.
Thanks,
Mark