Skip to content

Commit d308929

Browse files
jbachorikjbachorik
committed
Enable ASGCT by default on fairly safe J9 JDK versions
1 parent 4ca7f21 commit d308929

File tree

9 files changed

+257
-50
lines changed

9 files changed

+257
-50
lines changed

‎ddprof-lib/src/main/cpp/arguments.cpp‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
*/
1616

1717
#include "arguments.h"
18+
#include "vmEntry.h"
19+
1820
#include <limits.h>
1921
#include <stdio.h>
2022
#include <stdlib.h>
@@ -345,6 +347,14 @@ Error Arguments::parse(const char *args) {
345347
_event = EVENT_CPU;
346348
}
347349

350+
if (VM::isOpenJ9()) {
351+
if (_cstack == CSTACK_FP) {
352+
// J9 is compiled without FP
353+
// switch to DWARF for better results
354+
_cstack = CSTACK_DWARF;
355+
}
356+
}
357+
348358
return Error::OK;
349359
}
350360

‎ddprof-lib/src/main/cpp/j9Ext.h‎

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,9 @@
2020

2121
#include <jvmti.h>
2222

23+
#include "log.h"
2324
#include "vmEntry.h"
25+
#include "vmStructs.h"
2426

2527
#define JVMTI_EXT(f, ...) ((jvmtiError(*)(jvmtiEnv *, __VA_ARGS__))f)
2628

@@ -53,24 +55,44 @@ class J9Ext {
5355
static jvmtiExtensionFunction _GetAllStackTracesExtended;
5456

5557
public:
58+
static bool keepsJNIIDs() {
59+
char *flag_addr = (char *)JVMFlag::find("KeepJNIIDs", {JVMFlag::Type::Bool});
60+
return flag_addr != nullptr && *flag_addr;
61+
}
62+
5663
static bool can_use_ASGCT() {
64+
// as of 21.0.6 the use of ASGCT will lead to almost immediate crash
65+
// or livelock on J9
5766
return (VM::java_version() == 8 && VM::java_update_version() >= 362) ||
5867
(VM::java_version() == 11 && VM::java_update_version() >= 18) ||
5968
(VM::java_version() == 17 && VM::java_update_version() >= 6) ||
60-
(VM::java_version() >= 18);
61-
}
62-
63-
static bool is_jmethodid_safe() {
64-
return VM::java_version() == 8 ||
65-
(VM::java_version() == 11 && VM::java_update_version() >= 23) ||
66-
(VM::java_version() == 17 && VM::java_update_version() >= 11) ||
67-
(VM::java_version() == 21 && VM::java_update_version() >= 3) ||
68-
(VM::java_version() >= 22);
69+
(VM::java_version() >= 18 && VM::java_version() < 21);
6970
}
7071

7172
static bool is_jvmti_jmethodid_safe() {
7273
// only JDK 8 is safe to use jmethodID in JVMTI for deferred resolution
73-
return VM::java_version() == 8;
74+
// unless -XX:+KeepJNIIDs=true is provided
75+
return VM::java_version() == 8 || keepsJNIIDs();
76+
}
77+
78+
static bool shouldUseAsgct() {
79+
char *sampler = NULL;
80+
81+
jvmtiEnv *jvmti = VM::jvmti();
82+
jvmti->GetSystemProperty("dd.profiling.ddprof.j9.sampler", &sampler);
83+
84+
bool asgct = true;
85+
if (sampler != nullptr) {
86+
if (!strncmp("asgct", sampler, 5)) {
87+
asgct = true;
88+
} else if (!strncmp("jvmti", sampler, 5)) {
89+
asgct = false;
90+
} else {
91+
Log::warn("Invalid J9 sampler: %s, supported values are [jvmti, asgct]");
92+
}
93+
}
94+
jvmti->Deallocate((unsigned char *)sampler);
95+
return asgct;
7496
}
7597

7698
static bool initialize(jvmtiEnv *jvmti, const void *j9thread_self);

‎ddprof-lib/src/main/cpp/javaApi.cpp‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,14 @@ Java_com_datadoghq_profiler_JavaProfiler_execute0(JNIEnv *env, jobject unused,
109109
return NULL;
110110
}
111111

112+
extern "C" DLLEXPORT jstring JNICALL
113+
Java_com_datadoghq_profiler_JavaProfiler_getStatus0(JNIEnv* env,
114+
jobject unused) {
115+
char msg[2048];
116+
int ret = Profiler::instance()->status((char*)msg, sizeof(msg) - 1);
117+
return env->NewStringUTF(msg);
118+
}
119+
112120
extern "C" DLLEXPORT jlong JNICALL
113121
Java_com_datadoghq_profiler_JavaProfiler_getSamples(JNIEnv *env,
114122
jobject unused) {

‎ddprof-lib/src/main/cpp/profiler.cpp‎

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -990,11 +990,17 @@ Engine *Profiler::selectCpuEngine(Arguments &args) {
990990
return &noop_engine;
991991
} else if (args._cpu >= 0 || strcmp(args._event, EVENT_CPU) == 0) {
992992
if (VM::isOpenJ9()) {
993-
if (!J9Ext::is_jvmti_jmethodid_safe()) {
994-
Log::warn("Safe jmethodID access is not available on this JVM. Using "
995-
"CPU profiler on your own risk.");
993+
if (!J9Ext::shouldUseAsgct() || !J9Ext::can_use_ASGCT()) {
994+
if (!J9Ext::is_jvmti_jmethodid_safe()) {
995+
fprintf(stderr, "[ddprof] [WARN] Safe jmethodID access is not available on this JVM. Using "
996+
"CPU profiler on your own risk. Use -XX:+KeepJNIIDs=true JVM "
997+
"flag to make access to jmethodIDs safe, if your JVM supports it\n");
998+
return &noop_engine;
999+
}
1000+
TEST_LOG("J9[cpu]=jvmti");
1001+
return &j9_engine;
9961002
}
997-
return &j9_engine;
1003+
TEST_LOG("J9[cpu]=asgct");
9981004
}
9991005
return !ctimer.check(args)
10001006
? (Engine *)&ctimer
@@ -1017,14 +1023,18 @@ Engine *Profiler::selectWallEngine(Arguments &args) {
10171023
return &noop_engine;
10181024
}
10191025
if (VM::isOpenJ9()) {
1020-
if (args._wallclock_sampler == JVMTI) {
1026+
if (args._wallclock_sampler == JVMTI || !J9Ext::shouldUseAsgct() || !J9Ext::can_use_ASGCT()) {
10211027
if (!J9Ext::is_jvmti_jmethodid_safe()) {
1022-
Log::warn("Safe jmethodID access is not available on this JVM. Using "
1023-
"wallclock profiler on your own risk.");
1028+
fprintf(stderr, "[ddprof] [WARN] Safe jmethodID access is not available on this JVM. Using "
1029+
"wallclock profiler on your own risk. Use -XX:+KeepJNIIDs=true JVM "
1030+
"flag to make access to jmethodIDs safe, if your JVM supports it\n");
1031+
return &noop_engine;
10241032
}
10251033
j9_engine.sampleIdleThreads();
1034+
TEST_LOG("J9[wall]=jvmti");
10261035
return (Engine *)&j9_engine;
10271036
} else {
1037+
TEST_LOG("J9[wall]=asgct");
10281038
return (Engine *)&wall_asgct_engine;
10291039
}
10301040
}
@@ -1480,3 +1490,16 @@ int Profiler::lookupClass(const char *key, size_t length) {
14801490
// unable to lookup the class
14811491
return -1;
14821492
}
1493+
1494+
int Profiler::status(char* status, int max_len) {
1495+
return snprintf(status, max_len,
1496+
"== Java-Profiler Status ===\n"
1497+
" Running : %s\n"
1498+
" CPU Engine : %s\n"
1499+
" WallClock Engine : %s\n"
1500+
" Allocations : %s\n",
1501+
_state == RUNNING ? "true" : "false",
1502+
_cpu_engine != nullptr ? _cpu_engine->name() : "None",
1503+
_wall_engine != nullptr ? _wall_engine->name() : "None",
1504+
_alloc_engine != nullptr ? _alloc_engine->name() : "None");
1505+
}

‎ddprof-lib/src/main/cpp/profiler.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,7 @@ class Profiler {
146146
void updateJavaThreadNames();
147147
void updateNativeThreadNames();
148148
void mangle(const char *name, char *buf, size_t size);
149+
149150
Engine *selectCpuEngine(Arguments &args);
150151
Engine *selectWallEngine(Arguments &args);
151152
Engine *selectAllocEngine(Arguments &args);
@@ -180,6 +181,8 @@ class Profiler {
180181
return _instance;
181182
}
182183

184+
int status(char* status, int max_len);
185+
183186
u64 total_samples() { return _total_samples; }
184187
int max_stack_depth() { return _max_stack_depth; }
185188
time_t uptime() { return time(NULL) - _start_time; }

‎ddprof-lib/src/main/cpp/vmEntry.cpp‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -383,10 +383,6 @@ bool VM::initProfilerBridge(JavaVM *vm, bool attach) {
383383
*flag_addr = 1;
384384
}
385385
}
386-
char *flag_addr = (char *)JVMFlag::find("KeepJNIIDs", {JVMFlag::Type::Bool});
387-
if (flag_addr != NULL) {
388-
*flag_addr = 1;
389-
}
390386

391387
// if the user sets -XX:+UseAdaptiveGCBoundary we will just disable the
392388
// profiler to avoid the risk of crashing flag was made obsolete (inert) in 15

‎ddprof-lib/src/main/java/com/datadoghq/profiler/JavaProfiler.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,10 @@ public String getVersion() {
167167
}
168168
}
169169

170+
public String getStatus() {
171+
return getStatus0();
172+
}
173+
170174
/**
171175
* Execute an agent-compatible profiling command -
172176
* the comma-separated list of arguments described in arguments.cpp
@@ -472,4 +476,6 @@ public Map<String, Long> getDebugCounters() {
472476
private static native long tscFrequency0();
473477

474478
private static native void mallocArenaMax0(int max);
479+
480+
private static native String getStatus0();
475481
}

‎ddprof-test/src/test/java/com/datadoghq/profiler/ExternalLauncher.java‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,23 @@
22

33
public class ExternalLauncher {
44
public static void main(String[] args) throws Exception {
5-
if (args.length != 1) {
5+
if (args.length < 1) {
66
throw new RuntimeException();
77
}
88
if (args[0].equals("library")) {
99
JVMAccess.getInstance();
1010
} else if (args[0].equals("profiler")) {
11-
JavaProfiler.getInstance();
11+
JavaProfiler instance = JavaProfiler.getInstance();
12+
if (args.length == 2) {
13+
String commands = args[1];
14+
if (!commands.isEmpty()) {
15+
instance.execute(commands);
16+
}
17+
}
1218
}
19+
System.out.flush();
20+
System.err.flush();
21+
// wait for signal to exit
22+
System.in.read();
1323
}
1424
}

0 commit comments

Comments
 (0)