United StatesChange Country, Oracle Worldwide Web Sites Communities I am a... I want to...
Bug ID: JDK-7170638 enable support for dtrace compatible sdt probes on GNU/Linux
JDK-7170638 : enable support for dtrace compatible sdt probes on GNU/Linux

Details
Type:
Enhancement
Submit Date:
2012-05-22
Status:
Resolved
Updated Date:
2013-08-06
Project Name:
JDK
Resolved Date:
2012-10-09
Component:
hotspot
OS:
linux
Sub-Component:
runtime
CPU:
generic
Priority:
P3
Resolution:
Fixed
Affected Versions:
hs24
Fixed Versions:
hs25 (b05)

Related Reports
Backport:
Backport:
Backport:
Backport:
Relates:
Relates:
Relates:
Relates:
Relates:

Sub Tasks

Description
This bug created on behalf of Mark Wielaard from RedHat:

On 5/10/12 7:49 AM, Mark Wielaard wrote:
> Hi,
>
> The following three patches enable support for dtrace compatible sdt probes
> on GNU/Linux. Most of the support was already submitted earlier. These are
> just the last three things left in IcedTea that actually enable it in
> hotspot. It has been in production use for some time already.
>
> The patches could be applied all as one, or individually if you want to be
> able to test them independently.
>
> 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).
>
> The second patch introduces the DTRACE macros for use with the GNU/Linux
> SystemTap provided sys/sdt.h. Like Solaris this uses USDT1, but using
> macros instead of function declaractions. So it just adds a check for
> whether USDT1 is used with SOLARIS (HS_DTRACE_WORKAROUND_TAIL_CALL_BUG
> is only needed there) and defines HS_DTRACE_PROBE[N] for the LINUX case.
>
> The last patch enables the support in the build. I took the approach
> used in the make/solaris/dtrace.make and vm.make file with a test to
> see whether sys/sdt.h is available or not and printing a notice when
> they are not found. In IcedTea there is a more elaborate configure
> check to see if a small C++ program can be build with DTRACE probes
> in it, but that looked like overkill here.
>
> Patches in separate emails.
>
> Cheers,
>
> Mark
>
> The second patch adds the actual
>   src/share/vm/prims/jni.cpp        |  11 ++------
>   src/share/vm/utilities/dtrace.hpp |  50 ++++++++++++++++++++++++++++++++++++++-
>   make/linux/makefiles/dtrace.make  |  24 ++++++++++++++++++
>   make/linux/makefiles/vm.make      |   2 +-
>   4 files changed, 77 insertions(+), 10 deletions(-)

                                    

Comments
SUGGESTED FIX

The fix consists of the following 3 patches.

Patch 1: Use DTRACE_PROBE[N] in JNI Set and SetStatic Field

On 5/10/12 7:49 AM, Mark Wielaard wrote:
>   src/share/vm/prims/jni.cpp |  11 +++--------
>   1 files changed, 3 insertions(+), 8 deletions(-)
>
>
> # HG changeset patch
> # User Mark Wielaard<###@###.###>
> # Date 1336656680 -7200
> # Node ID ed003325d9977d3bd8b38afd5db5a97200b310af
> # Parent  2766551175a024338344350b9c279797b4a7d8ac
> XXXXXX: Use DTRACE_PROBE[N] in JNI Set and SetStatic Field.
> Summary: Don't use HS_DTRACE_PROBE_CDECL_N and HS_DTRACE_PROBE_N directly.
> Contributed-by: Mark Wielaard<###@###.###>
>
> diff -r 2766551175a0 -r ed003325d997 src/share/vm/prims/jni.cpp
> --- a/src/share/vm/prims/jni.cpp    Wed May 09 10:54:29 2012 -0700
> +++ b/src/share/vm/prims/jni.cpp    Thu May 10 15:31:20 2012 +0200
> @@ -1,5 +1,6 @@
>   /*
>    * Copyright (c) 1997, 2011, Oracle and/or its affiliates. All rights reserved.
> + * Copyright (c) 2012 Red Hat, Inc.
>    * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>    *
>    * This code is free software; you can redistribute it and/or modify it
> @@ -2818,10 +2819,7 @@
>   JNI_QUICK_ENTRY(void, jni_Set##Result##Field(JNIEnv *env, jobject obj, jfieldID fieldID, Argument value)) \
>     JNIWrapper("Set" XSTR(Result) "Field"); \
>   \
> -  HS_DTRACE_PROBE_CDECL_N(hotspot_jni, Set##Result##Field__entry, \
> -    ( JNIEnv*, jobject, jfieldID FP_SELECT_##Result(COMMA Argument,/*empty*/) ) ); \
> -  HS_DTRACE_PROBE_N(hotspot_jni, Set##Result##Field__entry, \
> -    ( env, obj, fieldID FP_SELECT_##Result(COMMA value,/*empty*/) ) ); \
> +  FP_SELECT_##Result(DTRACE_PROBE4(hotspot_jni, Set##Result##Field__entry, env, obj, fieldID, value),DTRACE_PROBE3(hotspot_jni, Set##Result##Field__entry, env, obj, fieldID)); \
>   \
>     oop o = JNIHandles::resolve_non_null(obj); \
>     klassOop k = o->klass(); \
> @@ -3128,10 +3126,7 @@
>   \
>   JNI_ENTRY(void, jni_SetStatic##Result##Field(JNIEnv *env, jclass clazz, jfieldID fieldID, Argument value)) \
>     JNIWrapper("SetStatic" XSTR(Result) "Field"); \
> -  HS_DTRACE_PROBE_CDECL_N(hotspot_jni, SetStatic##Result##Field__entry,\
> -    ( JNIEnv*, jclass, jfieldID FP_SELECT_##Result(COMMA Argument,/*empty*/) ) ); \
> -  HS_DTRACE_PROBE_N(hotspot_jni, SetStatic##Result##Field__entry, \
> -    ( env, clazz, fieldID FP_SELECT_##Result(COMMA value,/*empty*/) ) ); \
> +  FP_SELECT_##Result(DTRACE_PROBE4(hotspot_jni, SetStatic##Result##Field__entry, env, clazz, fieldID, value),DTRACE_PROBE3(hotspot_jni, SetStatic##Result##Field__entry, env, clazz, fieldID)); \
>   \
>     JNIid* id = jfieldIDWorkaround::from_static_jfieldID(fieldID); \
>     assert(id->is_static_field_id(), "invalid static field id"); \ 

Patch 2: Define DTRACE macros for use with GNU/Linux SystemTap sys/sdt.h

On 5/10/12 7:49 AM, Mark Wielaard wrote:
>   src/share/vm/utilities/dtrace.hpp |  50 ++++++++++++++++++++++++++++++++++++++-
>   1 files changed, 49 insertions(+), 1 deletions(-)
>
>
> # HG changeset patch
> # User Mark Wielaard<###@###.###>
> # Date 1336657104 -7200
> # Node ID ec675cf936d8eeefb11b7624c8f292fdd7e1f847
> # Parent  ed003325d9977d3bd8b38afd5db5a97200b310af
> XXXXXX: Define DTRACE macros for use with GNU/Linux SystemTap sys/sdt.h.
> Summary: LINUX also uses USDT1, but with macros instead of function decls.
> Contributed-by: Mark Wielaard<###@###.###>
>
> diff -r ed003325d997 -r ec675cf936d8 src/share/vm/utilities/dtrace.hpp
> --- a/src/share/vm/utilities/dtrace.hpp    Thu May 10 15:31:20 2012 +0200
> +++ b/src/share/vm/utilities/dtrace.hpp    Thu May 10 15:38:24 2012 +0200
> @@ -1,5 +1,6 @@
>   /*
>    * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved.
> + * Copyright (c) 2009, 2012 Red Hat, Inc.
>    * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>    *
>    * This code is free software; you can redistribute it and/or modify it
> @@ -32,12 +33,15 @@
>   #define DTRACE_ONLY(x) x
>   #define NOT_DTRACE(x)
>
> +#if defined(SOLARIS)
>   // Work around dtrace tail call bug 6672627 until it is fixed in solaris 10.
>   #define HS_DTRACE_WORKAROUND_TAIL_CALL_BUG() \
>     do { volatile size_t dtrace_workaround_tail_call_bug = 1; } while (0)
>
> -#if defined(SOLARIS)
>   #define USDT1 1
> +#elif defined(LINUX)
> +#define USDT1 1
> +#define HS_DTRACE_WORKAROUND_TAIL_CALL_BUG()
>   #elif defined(__APPLE__)
>   #define USDT2 1
>   #include<sys/types.h>
> @@ -63,6 +67,11 @@
>   #define DTRACE_PROBE3(a,b,c,d,e) {;}
>   #define DTRACE_PROBE4(a,b,c,d,e,f) {;}
>   #define DTRACE_PROBE5(a,b,c,d,e,f,g) {;}
> +#define DTRACE_PROBE6(a,b,c,d,e,f,g,h) {;}
> +#define DTRACE_PROBE7(a,b,c,d,e,f,g,h,i) {;}
> +#define DTRACE_PROBE8(a,b,c,d,e,f,g,h,i,j) {;}
> +#define DTRACE_PROBE9(a,b,c,d,e,f,g,h,i,j,k) {;}
> +#define DTRACE_PROBE10(a,b,c,d,e,f,g,h,i,j,k,l) {;}
>
>   #else /* USDT2 */
>
> @@ -76,10 +85,19 @@
>   #define HS_DTRACE_PROBE_FN(provider,name)\
>     __dtrace_##provider##___##name
>
> +#ifdef SOLARIS
> +// Solaris dtrace needs actual extern function decls.
>   #define HS_DTRACE_PROBE_DECL_N(provider,name,args) \
>     DTRACE_ONLY(extern "C" void HS_DTRACE_PROBE_FN(provider,name) args)
>   #define HS_DTRACE_PROBE_CDECL_N(provider,name,args) \
>     DTRACE_ONLY(extern void HS_DTRACE_PROBE_FN(provider,name) args)
> +#elif defined(LINUX)
> +// Systemtap dtrace compatible probes on GNU/Linux don't.
> +#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
>
>   /* Dtrace probe declarations */
>   #define HS_DTRACE_PROBE_DECL(provider,name) \
> @@ -118,6 +136,8 @@
>       uintptr_t,uintptr_t,uintptr_t))
>
>   /* Dtrace probe definitions */
> +#if defined(SOLARIS)
> +// Solaris dtrace uses actual function calls.
>   #define HS_DTRACE_PROBE_N(provider,name, args) \
>     DTRACE_ONLY(HS_DTRACE_PROBE_FN(provider,name) args)
>
> @@ -153,6 +173,34 @@
>     HS_DTRACE_PROBE_N(provider,name,((uintptr_t)a0,(uintptr_t)a1,(uintptr_t)a2,\
>       (uintptr_t)a3,(uintptr_t)a4,(uintptr_t)a5,(uintptr_t)a6,(uintptr_t)a7,\
>       (uintptr_t)a8,(uintptr_t)a9))
> +#elif defined(LINUX)
> +// Systemtap dtrace compatible probes on GNU/Linux use direct macros.
> +#define HS_DTRACE_PROBE(provider,name) HS_DTRACE_PROBE0(provider,name)
> +#define HS_DTRACE_PROBE0(provider,name)\
> +  DTRACE_PROBE(provider,name)
> +#define HS_DTRACE_PROBE1(provider,name,a0)\
> +  DTRACE_PROBE1(provider,name,a0)
> +#define HS_DTRACE_PROBE2(provider,name,a0,a1)\
> +  DTRACE_PROBE2(provider,name,a0,a1)
> +#define HS_DTRACE_PROBE3(provider,name,a0,a1,a2)\
> +  DTRACE_PROBE3(provider,name,a0,a1,a2)
> +#define HS_DTRACE_PROBE4(provider,name,a0,a1,a2,a3)\
> +  DTRACE_PROBE4(provider,name,a0,a1,a2,a3)
> +#define HS_DTRACE_PROBE5(provider,name,a0,a1,a2,a3,a4)\
> +  DTRACE_PROBE5(provider,name,a0,a1,a2,a3,a4)
> +#define HS_DTRACE_PROBE6(provider,name,a0,a1,a2,a3,a4,a5)\
> +  DTRACE_PROBE6(provider,name,a0,a1,a2,a3,a4,a5)
> +#define HS_DTRACE_PROBE7(provider,name,a0,a1,a2,a3,a4,a5,a6)\
> +  DTRACE_PROBE7(provider,name,a0,a1,a2,a3,a4,a5,a6)
> +#define HS_DTRACE_PROBE8(provider,name,a0,a1,a2,a3,a4,a5,a6,a7)\
> +  DTRACE_PROBE8(provider,name,a0,a1,a2,a3,a4,a5,a6,a7)
> +#define HS_DTRACE_PROBE9(provider,name,a0,a1,a2,a3,a4,a5,a6,a7,a8)\
> +  DTRACE_PROBE9(provider,name,a0,a1,a2,a3,a4,a5,a6,a7,a8)
> +#define HS_DTRACE_PROBE10(provider,name,a0,a1,a2,a3,a4,a5,a6,a7,a8)\
> +  DTRACE_PROBE10(provider,name,a0,a1,a2,a3,a4,a5,a6,a7,a8)
> +#else
> +#error "USDT1 enabled for unknown os"
> +#endif
>
>   #endif /* !USDT2 */

Patch 3: Enable dtrace compatible sdt probes on GNU/Linux build

On 5/10/12 7:49 AM, Mark Wielaard wrote:
>   make/linux/makefiles/dtrace.make |  24 ++++++++++++++++++++++++
>   make/linux/makefiles/vm.make     |   2 +-
>   2 files changed, 25 insertions(+), 1 deletions(-)
>
>
> # HG changeset patch
> # User Mark Wielaard<###@###.###>
> # Date 1336657292 -7200
> # Node ID 6a35858b331dca2ce76b6328ff57f893349303a0
> # Parent  ec675cf936d8eeefb11b7624c8f292fdd7e1f847
> XXXXXX: Enable dtrace compatible sdt probes on GNU/Linux builds.
> Summary: If sys/sdt.h is found, then enable dtrace compatible sdt probes.
> Contributed-by: Mark Wielaard<###@###.###>
>
> diff -r ec675cf936d8 -r 6a35858b331d make/linux/makefiles/dtrace.make
> --- a/make/linux/makefiles/dtrace.make    Thu May 10 15:38:24 2012 +0200
> +++ b/make/linux/makefiles/dtrace.make    Thu May 10 15:41:32 2012 +0200
> @@ -1,5 +1,6 @@
>   #
>   # Copyright (c) 2005, 2008, Oracle and/or its affiliates. All rights reserved.
> +# Copyright (c) 2012 Red Hat, Inc.
>   # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>   #
>   # This code is free software; you can redistribute it and/or modify it
> @@ -25,3 +26,26 @@
>   # Linux does not build jvm_db
>   LIBJVM_DB =
>
> +# But it does have a SystemTap dtrace compatible sys/sdt.h
> +ifneq ($(ALT_SDT_H),)
> +  SDT_H_FILE = $(ALT_SDT_H)
> +else
> +  SDT_H_FILE = /usr/include/sys/sdt.h
> +endif
> +DTRACE_ENABLED = $(shell test -f $(SDT_H_FILE)&&  echo $(SDT_H_FILE))
> +
> +ifneq ($(DTRACE_ENABLED),)
> +  CFLAGS += -DDTRACE_ENABLED
> +endif
> +
> +# Phone target used in vm.make build target to check whether enabled.
> +.PHONY: dtraceCheck
> +ifeq ($(DTRACE_ENABLED),)
> +dtraceCheck:
> +    $(QUIETLY) echo "**NOTICE** Dtrace support disabled $(SDT_H_FILE) not found"
> +else
> +dtraceCheck:
> +endif
> +
> +# It doesn't support HAVE_DTRACE_H though.
> +
> diff -r ec675cf936d8 -r 6a35858b331d make/linux/makefiles/vm.make
> --- a/make/linux/makefiles/vm.make    Thu May 10 15:38:24 2012 +0200
> +++ b/make/linux/makefiles/vm.make    Thu May 10 15:41:32 2012 +0200
> @@ -383,7 +383,7 @@
>
>   #----------------------------------------------------------------------
>
> -build: $(LIBJVM) $(LAUNCHER) $(LIBJSIG) $(LIBJVM_DB) $(BUILDLIBSAPROC) $(WB_JAR)
> +build: $(LIBJVM) $(LAUNCHER) $(LIBJSIG) $(LIBJVM_DB) $(BUILDLIBSAPROC) dtraceCheck $(WB_JAR)
>
>   install: install_jvm install_jsig install_saproc
                                     
2012-05-22
This bug is to only add the code to support OPENJDK.

Mikeal Vidstedt writes 9/28/2012 7:53 PM:

Let's see if I can summarize this:

* The functionality is only compiled into HotSpot if compiling on a system with sys/sdt.h and using GCC 4.5+
* That means that it will not be in the Oracle JDK short-term, because we (Oracle) are still compiling with GCC 4.3
* We (Oracle) will not be able to test this internally until we upgrade the tool chain (and potentially the distribution we use to compile)
** That also means there's a risk that we break the functionality as part of other changes, and RedHat and others will need to help submit patches as needed, but that's not really new
* Since it's not going to be in the Oracle JDK short-term I'm also less worried about the impact of integrating it
** We (Oracle) should make a mental note that when we upgrade the tool chain/build environment this functionality may be automatically enabled in the Oracle JDK and decide what level of testing we want to do
** We should also look at actually upgrading the tool chain to, among other things, get this functionality
* It'd be great to get some more tests of this functionality, but that's something we can work on as a follow up project

With this clarified I'm much less worried about picking up these patches and I think we should go ahead and get them integrated. Coleen - I'm assuming you'll take the lead on that?

Mark - Thanks for helping iron out the details around this and for your patience and understanding in the delays in finding resources to pick it up.

Cheers,
Mikael 
                                     
2012-10-03
Bug 8000613 is opened because this bug had hs24 as targeted version.  This bug is fixed in hs25 and we do not have plans to backport to hs24.
                                     
2012-10-09
Reopening for hs25.
                                     
2012-10-09
URL: http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/75982791ddb6
User: coleenp
Date: 2012-10-08 23:56:44 +0000
                                     
2012-10-09
URL:   http://hg.openjdk.java.net/hsx/hotspot-main/hotspot/rev/75982791ddb6
User:  coleenp
Date:  2012-10-11 02:50:42 +0000

                                     
2012-10-11
URL: http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/75982791ddb6
User: coleenp
Date: 2012-10-08 23:56:44 +0000
                                     
2012-10-09
URL:   http://hg.openjdk.java.net/hsx/hsx25/hotspot/rev/75982791ddb6
User:  amurillo
Date:  2012-10-12 23:14:51 +0000

                                     
2012-10-12



Hardware and Software, Engineered to Work Together