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

[Keith McGuigan]

On 5/23/2012 9:26 AM, Mark Wielaard wrote:
> On Wed, 2012-05-23 at 09:14 -0400, Keith McGuigan wrote:
>> Just a couple of quick comments/questions on the code:
>>
>> Why did we get rid of a couple of the declarations in jni.cpp?  Why
>> aren't they needed?
>
> Could you say which ones you think we got rid of? It was not my
> intention to get rid of anything. Maybe you are referring to the first
> patch? That came with the following description:
>
>> 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!

>> 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.

>> Has this been built on solaris, and is the dtrace functionality unchanged?
>
> I don't have access to solaris, the dtrace functionality should of
> course be unchanged.

Understood.  We'll just have to make sure that gets done somehow.

--
- Keith