Skip to content

Commit c8e0e50

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

File tree

11 files changed

+314
-129
lines changed

11 files changed

+314
-129
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: 26 additions & 9 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

@@ -54,25 +56,40 @@ class J9Ext {
5456

5557
public:
5658
static bool can_use_ASGCT() {
59+
// as of 21.0.6 the use of ASGCT will lead to almost immediate crash
60+
// or livelock on J9
5761
return (VM::java_version() == 8 && VM::java_update_version() >= 362) ||
5862
(VM::java_version() == 11 && VM::java_update_version() >= 18) ||
5963
(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);
64+
(VM::java_version() >= 18 && VM::java_version() < 21);
6965
}
7066

7167
static bool is_jvmti_jmethodid_safe() {
7268
// only JDK 8 is safe to use jmethodID in JVMTI for deferred resolution
69+
// unless -XX:+KeepJNIIDs=true is provided
7370
return VM::java_version() == 8;
7471
}
7572

73+
static bool shouldUseAsgct() {
74+
char *sampler = NULL;
75+
76+
jvmtiEnv *jvmti = VM::jvmti();
77+
jvmti->GetSystemProperty("dd.profiling.ddprof.j9.sampler", &sampler);
78+
79+
bool asgct = true;
80+
if (sampler != nullptr) {
81+
if (!strncmp("asgct", sampler, 5)) {
82+
asgct = true;
83+
} else if (!strncmp("jvmti", sampler, 5)) {
84+
asgct = false;
85+
} else {
86+
fprintf(stdout, "[ddprof] [WARN] Invalid J9 sampler: %s, supported values are [jvmti, asgct]", sampler);
87+
}
88+
}
89+
jvmti->Deallocate((unsigned char *)sampler);
90+
return asgct;
91+
}
92+
7693
static bool initialize(jvmtiEnv *jvmti, const void *j9thread_self);
7794

7895
static int GetOSThreadID(jthread thread) {

‎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: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -990,11 +990,16 @@ 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+
}
999+
TEST_LOG("J9[cpu]=jvmti");
1000+
return &j9_engine;
9961001
}
997-
return &j9_engine;
1002+
TEST_LOG("J9[cpu]=asgct");
9981003
}
9991004
return !ctimer.check(args)
10001005
? (Engine *)&ctimer
@@ -1017,14 +1022,17 @@ Engine *Profiler::selectWallEngine(Arguments &args) {
10171022
return &noop_engine;
10181023
}
10191024
if (VM::isOpenJ9()) {
1020-
if (args._wallclock_sampler == JVMTI) {
1025+
if (args._wallclock_sampler == JVMTI || !J9Ext::shouldUseAsgct() || !J9Ext::can_use_ASGCT()) {
10211026
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.");
1027+
fprintf(stderr, "[ddprof] [WARN] Safe jmethodID access is not available on this JVM. Using "
1028+
"wallclock profiler on your own risk. Use -XX:+KeepJNIIDs=true JVM "
1029+
"flag to make access to jmethodIDs safe, if your JVM supports it\n");
10241030
}
10251031
j9_engine.sampleIdleThreads();
1032+
TEST_LOG("J9[wall]=jvmti");
10261033
return (Engine *)&j9_engine;
10271034
} else {
1035+
TEST_LOG("J9[wall]=asgct");
10281036
return (Engine *)&wall_asgct_engine;
10291037
}
10301038
}
@@ -1480,3 +1488,16 @@ int Profiler::lookupClass(const char *key, size_t length) {
14801488
// unable to lookup the class
14811489
return -1;
14821490
}
1491+
1492+
int Profiler::status(char* status, int max_len) {
1493+
return snprintf(status, max_len,
1494+
"== Java-Profiler Status ===\n"
1495+
" Running : %s\n"
1496+
" CPU Engine : %s\n"
1497+
" WallClock Engine : %s\n"
1498+
" Allocations : %s\n",
1499+
_state == RUNNING ? "true" : "false",
1500+
_cpu_engine != nullptr ? _cpu_engine->name() : "None",
1501+
_wall_engine != nullptr ? _wall_engine->name() : "None",
1502+
_alloc_engine != nullptr ? _alloc_engine->name() : "None");
1503+
}

‎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/AbstractProcessProfilerTest.java‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
package com.datadoghq.profiler;
2+
3+
import java.io.BufferedReader;
4+
import java.io.IOException;
5+
import java.io.InputStreamReader;
6+
import java.util.ArrayList;
7+
import java.util.List;
8+
import java.util.concurrent.TimeUnit;
9+
import java.util.function.Function;
10+
11+
import static org.junit.jupiter.api.Assertions.*;
12+
13+
14+
public abstract class AbstractProcessProfilerTest {
15+
protected final boolean launch(String target, List<String> jvmArgs, String commands, Function<String, Boolean> onStdoutLine, Function<String, Boolean> onStderrLine) throws Exception {
16+
String javaHome = System.getenv("JAVA_TEST_HOME");
17+
if (javaHome == null) {
18+
javaHome = System.getenv("JAVA_HOME");
19+
}
20+
if (javaHome == null) {
21+
javaHome = System.getProperty("java.home");
22+
}
23+
assertNotNull(javaHome);
24+
25+
List<String> args = new ArrayList<>();
26+
args.add(javaHome + "/bin/java");
27+
args.addAll(jvmArgs);
28+
args.add("-cp");
29+
args.add(System.getProperty("java.class.path"));
30+
args.add(ExternalLauncher.class.getName());
31+
args.add(target);
32+
if (commands != null && !commands.isEmpty()) {
33+
args.add(commands);
34+
}
35+
36+
ProcessBuilder pb = new ProcessBuilder(args);
37+
Process p = pb.start();
38+
Thread stdoutReader = new Thread(() -> {
39+
Function<String, Boolean> lineProcessor = onStdoutLine != null ? onStdoutLine : l -> true;
40+
try (BufferedReader br = new BufferedReader(new InputStreamReader(p.getInputStream()))) {
41+
String line;
42+
while ((line = br.readLine()) != null) {
43+
System.out.println("[out] " + line);
44+
if (!lineProcessor.apply(line)) {
45+
try {
46+
p.getOutputStream().write(1);
47+
p.getOutputStream().flush();
48+
} catch (IOException ignored) {
49+
}
50+
} else {
51+
if (line.contains("[ready]")) {
52+
p.getOutputStream().write(1);
53+
p.getOutputStream().flush();
54+
}
55+
}
56+
}
57+
} catch (IOException ignored) {
58+
} catch (Exception e) {
59+
throw new RuntimeException(e);
60+
}
61+
}, "stdout-reader");
62+
Thread stderrReader = new Thread(() -> {
63+
Function<String, Boolean> lineProcessor = onStderrLine != null ? onStderrLine : l -> true;
64+
try (BufferedReader br = new BufferedReader(new InputStreamReader(p.getErrorStream()))) {
65+
String line;
66+
while ((line = br.readLine()) != null) {
67+
System.out.println("[err] " + line);
68+
if (!lineProcessor.apply(line)) {
69+
try {
70+
p.getOutputStream().write(1);
71+
p.getOutputStream().flush();
72+
} catch (IOException ignored) {
73+
}
74+
}
75+
}
76+
} catch (IOException ignored) {
77+
} catch (Exception e) {
78+
throw new RuntimeException(e);
79+
}
80+
}, "stderr-reader");
81+
82+
stdoutReader.setDaemon(true);
83+
stderrReader.setDaemon(true);
84+
85+
stdoutReader.start();
86+
stderrReader.start();
87+
88+
boolean val = p.waitFor(10, TimeUnit.SECONDS);
89+
if (!val) {
90+
p.destroyForcibly();
91+
}
92+
return val;
93+
}
94+
}

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

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,27 @@
22

33
public class ExternalLauncher {
44
public static void main(String[] args) throws Exception {
5-
if (args.length != 1) {
6-
throw new RuntimeException();
7-
}
8-
if (args[0].equals("library")) {
9-
JVMAccess.getInstance();
10-
} else if (args[0].equals("profiler")) {
11-
JavaProfiler.getInstance();
5+
try {
6+
if (args.length < 1) {
7+
throw new RuntimeException();
8+
}
9+
if (args[0].equals("library")) {
10+
JVMAccess.getInstance();
11+
} else if (args[0].equals("profiler")) {
12+
JavaProfiler instance = JavaProfiler.getInstance();
13+
if (args.length == 2) {
14+
String commands = args[1];
15+
if (!commands.isEmpty()) {
16+
instance.execute(commands);
17+
}
18+
}
19+
}
20+
} finally {
21+
System.out.println("[ready]");
22+
System.out.flush();
23+
System.err.flush();
1224
}
25+
// wait for signal to exit
26+
System.in.read();
1327
}
1428
}

0 commit comments

Comments
 (0)