Skip to content

Commit

Permalink
Enable ASGCT by default on fairly safe J9 JDK versions
Browse files Browse the repository at this point in the history
  • Loading branch information
jbachorik committed Feb 14, 2025
1 parent 4ca7f21 commit 6880d41
Show file tree
Hide file tree
Showing 11 changed files with 317 additions and 127 deletions.
10 changes: 10 additions & 0 deletions ddprof-lib/src/main/cpp/arguments.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
*/

#include "arguments.h"
#include "vmEntry.h"

#include <limits.h>
#include <stdio.h>
#include <stdlib.h>
Expand Down Expand Up @@ -345,6 +347,14 @@ Error Arguments::parse(const char *args) {
_event = EVENT_CPU;
}

if (VM::isOpenJ9()) {
if (_cstack == CSTACK_FP) {
// J9 is compiled without FP
// switch to DWARF for better results
_cstack = CSTACK_DWARF;
}
}

return Error::OK;
}

Expand Down
35 changes: 26 additions & 9 deletions ddprof-lib/src/main/cpp/j9Ext.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@

#include <jvmti.h>

#include "log.h"
#include "vmEntry.h"
#include "vmStructs.h"

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

Expand Down Expand Up @@ -54,25 +56,40 @@ class J9Ext {

public:
static bool can_use_ASGCT() {
// as of 21.0.6 the use of ASGCT will lead to almost immediate crash
// or livelock on J9
return (VM::java_version() == 8 && VM::java_update_version() >= 362) ||
(VM::java_version() == 11 && VM::java_update_version() >= 18) ||
(VM::java_version() == 17 && VM::java_update_version() >= 6) ||
(VM::java_version() >= 18);
}

static bool is_jmethodid_safe() {
return VM::java_version() == 8 ||
(VM::java_version() == 11 && VM::java_update_version() >= 23) ||
(VM::java_version() == 17 && VM::java_update_version() >= 11) ||
(VM::java_version() == 21 && VM::java_update_version() >= 3) ||
(VM::java_version() >= 22);
(VM::java_version() >= 18 && VM::java_version() < 21);
}

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

static bool shouldUseAsgct() {
char *sampler = NULL;

jvmtiEnv *jvmti = VM::jvmti();
jvmti->GetSystemProperty("dd.profiling.ddprof.j9.sampler", &sampler);

bool asgct = true;
if (sampler != nullptr) {
if (!strncmp("asgct", sampler, 5)) {
asgct = true;
} else if (!strncmp("jvmti", sampler, 5)) {
asgct = false;
} else {
fprintf(stdout, "[ddprof] [WARN] Invalid J9 sampler: %s, supported values are [jvmti, asgct]", sampler);
}
}
jvmti->Deallocate((unsigned char *)sampler);
return asgct;
}

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

static int GetOSThreadID(jthread thread) {
Expand Down
8 changes: 8 additions & 0 deletions ddprof-lib/src/main/cpp/javaApi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,14 @@ Java_com_datadoghq_profiler_JavaProfiler_execute0(JNIEnv *env, jobject unused,
return NULL;
}

extern "C" DLLEXPORT jstring JNICALL
Java_com_datadoghq_profiler_JavaProfiler_getStatus0(JNIEnv* env,
jobject unused) {
char msg[2048];
int ret = Profiler::instance()->status((char*)msg, sizeof(msg) - 1);
return env->NewStringUTF(msg);
}

extern "C" DLLEXPORT jlong JNICALL
Java_com_datadoghq_profiler_JavaProfiler_getSamples(JNIEnv *env,
jobject unused) {
Expand Down
35 changes: 28 additions & 7 deletions ddprof-lib/src/main/cpp/profiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -990,11 +990,16 @@ Engine *Profiler::selectCpuEngine(Arguments &args) {
return &noop_engine;
} else if (args._cpu >= 0 || strcmp(args._event, EVENT_CPU) == 0) {
if (VM::isOpenJ9()) {
if (!J9Ext::is_jvmti_jmethodid_safe()) {
Log::warn("Safe jmethodID access is not available on this JVM. Using "
"CPU profiler on your own risk.");
if (!J9Ext::shouldUseAsgct() || !J9Ext::can_use_ASGCT()) {
if (!J9Ext::is_jvmti_jmethodid_safe()) {
fprintf(stderr, "[ddprof] [WARN] Safe jmethodID access is not available on this JVM. Using "
"CPU profiler on your own risk. Use -XX:+KeepJNIIDs=true JVM "
"flag to make access to jmethodIDs safe, if your JVM supports it\n");
}
TEST_LOG("J9[cpu]=jvmti");
return &j9_engine;
}
return &j9_engine;
TEST_LOG("J9[cpu]=asgct");
}
return !ctimer.check(args)
? (Engine *)&ctimer
Expand All @@ -1017,14 +1022,17 @@ Engine *Profiler::selectWallEngine(Arguments &args) {
return &noop_engine;
}
if (VM::isOpenJ9()) {
if (args._wallclock_sampler == JVMTI) {
if (args._wallclock_sampler == JVMTI || !J9Ext::shouldUseAsgct() || !J9Ext::can_use_ASGCT()) {
if (!J9Ext::is_jvmti_jmethodid_safe()) {
Log::warn("Safe jmethodID access is not available on this JVM. Using "
"wallclock profiler on your own risk.");
fprintf(stderr, "[ddprof] [WARN] Safe jmethodID access is not available on this JVM. Using "
"wallclock profiler on your own risk. Use -XX:+KeepJNIIDs=true JVM "
"flag to make access to jmethodIDs safe, if your JVM supports it\n");
}
j9_engine.sampleIdleThreads();
TEST_LOG("J9[wall]=jvmti");
return (Engine *)&j9_engine;
} else {
TEST_LOG("J9[wall]=asgct");
return (Engine *)&wall_asgct_engine;
}
}
Expand Down Expand Up @@ -1480,3 +1488,16 @@ int Profiler::lookupClass(const char *key, size_t length) {
// unable to lookup the class
return -1;
}

int Profiler::status(char* status, int max_len) {
return snprintf(status, max_len,
"== Java-Profiler Status ===\n"
" Running : %s\n"
" CPU Engine : %s\n"
" WallClock Engine : %s\n"
" Allocations : %s\n",
_state == RUNNING ? "true" : "false",
_cpu_engine != nullptr ? _cpu_engine->name() : "None",
_wall_engine != nullptr ? _wall_engine->name() : "None",
_alloc_engine != nullptr ? _alloc_engine->name() : "None");
}
3 changes: 3 additions & 0 deletions ddprof-lib/src/main/cpp/profiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ class Profiler {
void updateJavaThreadNames();
void updateNativeThreadNames();
void mangle(const char *name, char *buf, size_t size);

Engine *selectCpuEngine(Arguments &args);
Engine *selectWallEngine(Arguments &args);
Engine *selectAllocEngine(Arguments &args);
Expand Down Expand Up @@ -180,6 +181,8 @@ class Profiler {
return _instance;
}

int status(char* status, int max_len);

u64 total_samples() { return _total_samples; }
int max_stack_depth() { return _max_stack_depth; }
time_t uptime() { return time(NULL) - _start_time; }
Expand Down
4 changes: 0 additions & 4 deletions ddprof-lib/src/main/cpp/vmEntry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -383,10 +383,6 @@ bool VM::initProfilerBridge(JavaVM *vm, bool attach) {
*flag_addr = 1;
}
}
char *flag_addr = (char *)JVMFlag::find("KeepJNIIDs", {JVMFlag::Type::Bool});
if (flag_addr != NULL) {
*flag_addr = 1;
}

// if the user sets -XX:+UseAdaptiveGCBoundary we will just disable the
// profiler to avoid the risk of crashing flag was made obsolete (inert) in 15
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,10 @@ public String getVersion() {
}
}

public String getStatus() {
return getStatus0();
}

/**
* Execute an agent-compatible profiling command -
* the comma-separated list of arguments described in arguments.cpp
Expand Down Expand Up @@ -472,4 +476,6 @@ public Map<String, Long> getDebugCounters() {
private static native long tscFrequency0();

private static native void mallocArenaMax0(int max);

private static native String getStatus0();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
package com.datadoghq.profiler;

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.TimeUnit;
import java.util.function.Function;

import static org.junit.jupiter.api.Assertions.*;


public abstract class AbstractProcessProfilerTest {
protected final boolean launch(String target, List<String> jvmArgs, String commands, Function<String, Boolean> onStdoutLine, Function<String, Boolean> onStderrLine) throws Exception {
String javaHome = System.getenv("JAVA_TEST_HOME");
if (javaHome == null) {
javaHome = System.getenv("JAVA_HOME");
}
if (javaHome == null) {
javaHome = System.getProperty("java.home");
}
assertNotNull(javaHome);

List<String> args = new ArrayList<>();
args.add(javaHome + "/bin/java");
args.addAll(jvmArgs);
args.add("-cp");
args.add(System.getProperty("java.class.path"));
args.add(ExternalLauncher.class.getName());
args.add(target);
if (commands != null && !commands.isEmpty()) {
args.add(commands);
}

ProcessBuilder pb = new ProcessBuilder(args);
Process p = pb.start();
Thread stdoutReader = new Thread(() -> {
Function<String, Boolean> lineProcessor = onStdoutLine != null ? onStdoutLine : l -> true;
try (BufferedReader br = new BufferedReader(new InputStreamReader(p.getInputStream()))) {
String line;
while ((line = br.readLine()) != null) {
System.out.println("[out] " + line);
if (!lineProcessor.apply(line)) {
try {
p.getOutputStream().write(1);
p.getOutputStream().flush();
} catch (IOException ignored) {
}
} else {
if (line.contains("[ready]")) {
p.getOutputStream().write(1);
p.getOutputStream().flush();
}
}
}
} catch (IOException ignored) {
} catch (Exception e) {
throw new RuntimeException(e);
}
}, "stdout-reader");
Thread stderrReader = new Thread(() -> {
Function<String, Boolean> lineProcessor = onStderrLine != null ? onStderrLine : l -> true;
try (BufferedReader br = new BufferedReader(new InputStreamReader(p.getErrorStream()))) {
String line;
while ((line = br.readLine()) != null) {
System.out.println("[err] " + line);
if (!lineProcessor.apply(line)) {
try {
p.getOutputStream().write(1);
p.getOutputStream().flush();
} catch (IOException ignored) {
}
}
}
} catch (IOException ignored) {
} catch (Exception e) {
throw new RuntimeException(e);
}
}, "stderr-reader");

stdoutReader.setDaemon(true);
stderrReader.setDaemon(true);

stdoutReader.start();
stderrReader.start();

boolean val = p.waitFor(10, TimeUnit.SECONDS);
if (!val) {
p.destroyForcibly();
}
return val;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,27 @@

public class ExternalLauncher {
public static void main(String[] args) throws Exception {
if (args.length != 1) {
throw new RuntimeException();
}
if (args[0].equals("library")) {
JVMAccess.getInstance();
} else if (args[0].equals("profiler")) {
JavaProfiler.getInstance();
try {
if (args.length < 1) {
throw new RuntimeException();
}
if (args[0].equals("library")) {
JVMAccess.getInstance();
} else if (args[0].equals("profiler")) {
JavaProfiler instance = JavaProfiler.getInstance();
if (args.length == 2) {
String commands = args[1];
if (!commands.isEmpty()) {
instance.execute(commands);
}
}
}
} finally {
System.out.println("[ready]");
System.out.flush();
System.err.flush();
}
// wait for signal to exit
System.in.read();
}
}
Loading

0 comments on commit 6880d41

Please sign in to comment.