[PATCH 0 of 3] Add support for dtrace compatible sdt probes on GNU/Linux

[Mark Wielaard]

On Wed, 2012-05-23 at 09:42 -0400, Keith McGuigan wrote:
> >> The first patch is just a consistency cleanup patch. The JNI Set and
> >> SetStatic Field methods used HS_DTRACE_PROBE_CDECL_N and HS_DTRACE_PROBE_N
> >> directly instead of just using DTRACE_PROBE[N] like all other JNI methods.
> >> This doesn't matter for the Solaris macros, but on GNU/Linux the macros
> >> don't use direct function declarations (which is introduced in the second
> >> patch).
> 
> Ok, might be that it's fine.  I don't remember if there was a reason 
> that SetStatic was different from the other JNI calls, but it certainly 
> could just have been an omission in refactoring or something.  We just 
> need to make sure that it still works (solaris-dtrace-wise).  It's 
> certainly better this way!

Yeah, that is why I originally split out the patches, so that anything
that might potentially impact other architectures could be applied and
tested separately. Especially since I don't have access to solaris.

> >> dtrace.hpp should be refactored (if possible) into the os-specific
> >> subdirectories, instead of using #ifdef<OS>  macros in the header.
> >> I.e., you might have to make a src/solaris/vm/dtrace_solaris.hpp and
> >> src/linux/vm/dtrace_linux.hpp file and include it from here.
> >
> > I rather not, the code should be identical except for two small details
> > between solaris and gnu/linux as the patch shows (and one is just a hack
> > for a solaris tailcall bug). The apple/macos part looks very different
> > and could be split out, but that was already there.
> 
> I refer to the redefinition blocks of HS_DTRACE_PROBEx, which I don't 
> consider a small detail (though the solaris tailcall thing certainly 
> is).
>
>   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?

Thanks,

Mark