Request for review 7170638: Use DTRACE_PROBE[N] in JNI Set and SetStatic Field.

[Coleen Phillimore]

Thank you for the further review.   A couple comments inline.

On 10/5/2012 7:00 AM, Mark Wielaard wrote:
> On Thu, 2012-10-04 at 17:30 -0400, Coleen Phillimore wrote:
>> The other change was that dtrace.hpp didn't build on windows.  The linux
>> definitions expand to null for windows.
>> -+#elif defined(LINUX)
>> ++#else
>>   +// Systemtap dtrace compatible probes on GNU/Linux don't.
>> ++// If dtrace is disabled this macro becomes NULL
>>   +#define HS_DTRACE_PROBE_DECL_N(provider,name,args)
>>   +#define HS_DTRACE_PROBE_CDECL_N(provider,name,args)
>> -+#else
>> -+#error "USDT1 enabled for unknown os"
>>   +#endif
> I haven't had time to do a full rebuild here, but that change should be
> fine. I am a little surprised the windows build picks up the definitions
> from the USDT1/LINUX part. But empty defines should be fine. I don't
> have Windows around though, so cannot test myself.

I wasn't completely thrilled with this but the Hotspot source uses the 
USDT1 macros if USDT2 isn't defined and that's what windows was 
compiling, so windows shared the definitions of USDT1 macros with 
Linux.   It would be better if null macros were outside the 
DTRACE_ENABLED condition, but that copies all of the USDT1 macros 
again.  Someone should clean this up.
>
>> open webrev at http://cr.openjdk.java.net/~coleenp/7170638_5/
> Also thanks for picking up the buildtree.make changes for bsd and
> solaris. Note that this version doesn't include the new testcase, don't
> know if that was intentional?

We can add testcases when BSD is supported (it's not yet).

Thanks,
Coleen
>
> Cheers,
>
> Mark