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
