[PATCH 0 of 3] Add support for dtrace compatible sdt probes on GNU/Linux
keith.mcguigan at oracle.com
Wed May 23 06:42:04 PDT 2012
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
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.
More information about the hotspot-dev