Skip to content

[PROF-12714] Add CriticalNative support #282

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
+59 −29
Open

[PROF-12714] Add CriticalNative support #282

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4feff30
[PROF-12714] Add CriticalNative support
zhengyu123 Oct 10, 2025
6cfd40f
Disable CriticalJNINatives on aarch64
zhengyu123 Oct 24, 2025
19b54db
Remove none performance critical variants and add comments
zhengyu123 Oct 29, 2025
18c788e
Remove more none performance critical CriticalJNINatives methods
zhengyu123 Oct 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 43 additions & 25 deletions ddprof-lib/src/main/cpp/javaApi.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ Java_com_datadoghq_profiler_JavaProfiler_stop0(JNIEnv *env, jobject unused) {
}

extern "C" DLLEXPORT jint JNICALL
Java_com_datadoghq_profiler_JavaProfiler_getTid0(JNIEnv *env, jobject unused) {
Java_com_datadoghq_profiler_JavaProfiler_getTid0(JNIEnv *env, jclass unused) {
return OS::threadId();
}

Expand Down Expand Up @@ -113,22 +113,26 @@ Java_com_datadoghq_profiler_JavaProfiler_execute0(JNIEnv *env, jobject unused,

extern "C" DLLEXPORT jstring JNICALL
Java_com_datadoghq_profiler_JavaProfiler_getStatus0(JNIEnv* env,
jobject unused) {
jclass 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) {
jclass unused) {
return (jlong)Profiler::instance()->total_samples();
}

// some duplication between add and remove, though we want to avoid having an extra branch in the hot path

// JavaCritical is faster JNI, but more restrictive - parameters and return value have to be
// primitives or arrays of primitive types.
// We direct corresponding JNI calls to JavaCritical to make sure the parameters/return value
// still compatible in the event of signature changes in the future.
extern "C" DLLEXPORT void JNICALL
Java_com_datadoghq_profiler_JavaProfiler_filterThreadAdd0(JNIEnv *env,
jobject unused) {
JavaCritical_com_datadoghq_profiler_JavaProfiler_filterThreadAdd0() {
Comment on lines 134 to +135
Copy link
Member

@ivoanjo ivoanjo Oct 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: Should we maybe declare these as inline to softly suggest to the compiler that Java_com_datadoghq_profiler_JavaProfiler_filterThreadAdd0 should inline this, not call into it?

(Or does that not work very well anyway?)

ProfiledThread *current = ProfiledThread::current();
if (unlikely(current == nullptr)) {
assert(false);
Expand Down Expand Up @@ -158,8 +162,7 @@ Java_com_datadoghq_profiler_JavaProfiler_filterThreadAdd0(JNIEnv *env,
}

extern "C" DLLEXPORT void JNICALL
Java_com_datadoghq_profiler_JavaProfiler_filterThreadRemove0(JNIEnv *env,
jobject unused) {
JavaCritical_com_datadoghq_profiler_JavaProfiler_filterThreadRemove0() {
ProfiledThread *current = ProfiledThread::current();
if (unlikely(current == nullptr)) {
assert(false);
Expand All @@ -173,7 +176,7 @@ Java_com_datadoghq_profiler_JavaProfiler_filterThreadRemove0(JNIEnv *env,
if (unlikely(!thread_filter->enabled())) {
return;
}

int slot_id = current->filterSlotId();
if (unlikely(slot_id == -1)) {
// Thread doesn't have a slot ID yet - nothing to remove
Expand All @@ -182,10 +185,23 @@ Java_com_datadoghq_profiler_JavaProfiler_filterThreadRemove0(JNIEnv *env,
thread_filter->remove(slot_id);
}


extern "C" DLLEXPORT void JNICALL
Java_com_datadoghq_profiler_JavaProfiler_filterThreadAdd0(JNIEnv *env,
jclass unused) {
JavaCritical_com_datadoghq_profiler_JavaProfiler_filterThreadAdd0();
}

extern "C" DLLEXPORT void JNICALL
Java_com_datadoghq_profiler_JavaProfiler_filterThreadRemove0(JNIEnv *env,
jclass unused) {
JavaCritical_com_datadoghq_profiler_JavaProfiler_filterThreadRemove0();
}

// Backward compatibility for existing code
extern "C" DLLEXPORT void JNICALL
Java_com_datadoghq_profiler_JavaProfiler_filterThread0(JNIEnv *env,
jobject unused,
jclass unused,
jboolean enable) {
ProfiledThread *current = ProfiledThread::current();
if (unlikely(current == nullptr)) {
Expand All @@ -200,7 +216,7 @@ Java_com_datadoghq_profiler_JavaProfiler_filterThread0(JNIEnv *env,
if (unlikely(!thread_filter->enabled())) {
return;
}

int slot_id = current->filterSlotId();
if (unlikely(slot_id == -1)) {
if (enable) {
Expand All @@ -213,11 +229,11 @@ Java_com_datadoghq_profiler_JavaProfiler_filterThread0(JNIEnv *env,
return;
}
}

if (unlikely(slot_id == -1)) {
return; // Failed to register thread
}

if (enable) {
thread_filter->add(tid, slot_id);
} else {
Expand All @@ -227,7 +243,7 @@ Java_com_datadoghq_profiler_JavaProfiler_filterThread0(JNIEnv *env,

extern "C" DLLEXPORT jobject JNICALL
Java_com_datadoghq_profiler_JavaProfiler_getContextPage0(JNIEnv *env,
jobject unused,
jclass unused,
jint tid) {
ContextPage page = Contexts::getPage((int)tid);
if (page.storage == 0) {
Expand All @@ -238,21 +254,22 @@ Java_com_datadoghq_profiler_JavaProfiler_getContextPage0(JNIEnv *env,

extern "C" DLLEXPORT jlong JNICALL
Java_com_datadoghq_profiler_JavaProfiler_getContextPageOffset0(JNIEnv *env,
jobject unused,
jclass unused,
jint tid) {
ContextPage page = Contexts::getPage((int)tid);
return (jlong)page.storage;
}

extern "C" DLLEXPORT jint JNICALL
Java_com_datadoghq_profiler_JavaProfiler_getMaxContextPages0(JNIEnv *env,
jobject unused) {
jclass unused) {
return (jint)Contexts::getMaxPages();
}


extern "C" DLLEXPORT jboolean JNICALL
Java_com_datadoghq_profiler_JavaProfiler_recordTrace0(
JNIEnv *env, jobject unused, jlong rootSpanId, jstring endpoint,
JNIEnv *env, jclass unused, jlong rootSpanId, jstring endpoint,
jstring operation, jint sizeLimit) {
JniString endpoint_str(env, endpoint);
u32 endpointLabel = Profiler::instance()->stringLabelMap()->bounded_lookup(
Expand All @@ -274,7 +291,7 @@ Java_com_datadoghq_profiler_JavaProfiler_recordTrace0(

extern "C" DLLEXPORT jint JNICALL
Java_com_datadoghq_profiler_JavaProfiler_registerConstant0(JNIEnv *env,
jobject unused,
jclass unused,
jstring value) {
JniString value_str(env, value);
u32 encoding = Profiler::instance()->contextValueMap()->bounded_lookup(
Expand All @@ -283,15 +300,15 @@ Java_com_datadoghq_profiler_JavaProfiler_registerConstant0(JNIEnv *env,
}

extern "C" DLLEXPORT void JNICALL
Java_com_datadoghq_profiler_JavaProfiler_dump0(JNIEnv *env, jobject unused,
Java_com_datadoghq_profiler_JavaProfiler_dump0(JNIEnv *env, jclass unused,
jstring path) {
JniString path_str(env, path);
Profiler::instance()->dump(path_str.c_str(), path_str.length());
}

extern "C" DLLEXPORT jobject JNICALL
Java_com_datadoghq_profiler_JavaProfiler_getDebugCounters0(JNIEnv *env,
jobject unused) {
jclass unused) {
#ifdef COUNTERS
return env->NewDirectByteBuffer((void *)Counters::getCounters(),
(jlong)Counters::size());
Expand All @@ -302,7 +319,7 @@ Java_com_datadoghq_profiler_JavaProfiler_getDebugCounters0(JNIEnv *env,

extern "C" DLLEXPORT jobjectArray JNICALL
Java_com_datadoghq_profiler_JavaProfiler_describeDebugCounters0(
JNIEnv *env, jobject unused) {
JNIEnv *env, jclass unused) {
#ifdef COUNTERS
std::vector<const char *> counter_names = Counters::describeCounters();
jobjectArray array = (jobjectArray)env->NewObjectArray(
Expand All @@ -320,7 +337,7 @@ Java_com_datadoghq_profiler_JavaProfiler_describeDebugCounters0(

extern "C" DLLEXPORT void JNICALL
Java_com_datadoghq_profiler_JavaProfiler_recordSettingEvent0(
JNIEnv *env, jobject unused, jstring name, jstring value, jstring unit) {
JNIEnv *env, jclass unused, jstring name, jstring value, jstring unit) {
int tid = ProfiledThread::currentTid();
if (tid < 0) {
return;
Expand All @@ -343,7 +360,7 @@ static int dictionarizeClassName(JNIEnv* env, jstring className) {

extern "C" DLLEXPORT void JNICALL
Java_com_datadoghq_profiler_JavaProfiler_recordQueueEnd0(
JNIEnv *env, jobject unused, jlong startTime, jlong endTime, jstring task,
JNIEnv *env, jclass unused, jlong startTime, jlong endTime, jstring task,
jstring scheduler, jthread origin, jstring queueType, jint queueLength) {
int tid = ProfiledThread::currentTid();
if (tid < 0) {
Expand Down Expand Up @@ -379,19 +396,20 @@ Java_com_datadoghq_profiler_JavaProfiler_recordQueueEnd0(

extern "C" DLLEXPORT jlong JNICALL
Java_com_datadoghq_profiler_JavaProfiler_currentTicks0(JNIEnv *env,
jobject unused) {
jclass unused) {
return TSC::ticks();
}

extern "C" DLLEXPORT jlong JNICALL
Java_com_datadoghq_profiler_JavaProfiler_tscFrequency0(JNIEnv *env,
jobject unused) {
jclass unused) {
return TSC::frequency();
}


extern "C" DLLEXPORT void JNICALL
Java_com_datadoghq_profiler_JavaProfiler_mallocArenaMax0(JNIEnv *env,
jobject unused,
jclass unused,
jint maxArenas) {
ddprof::OS::mallocArenaMax(maxArenas);
}
Expand Down
11 changes: 11 additions & 0 deletions ddprof-lib/src/main/cpp/vmStructs_dd.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ namespace ddprof {
initOffsets();
initJvmFunctions();
initUnsafeFunctions();
initCriticalJNINatives();
}

void VMStructs_::initOffsets() {
Expand Down Expand Up @@ -98,6 +99,16 @@ namespace ddprof {
}
}

void VMStructs_::initCriticalJNINatives() {
#ifdef __aarch64__
// aarch64 does not support CriticalJNINatives
JVMFlag* flag = JVMFlag::find("CriticalJNINatives", {JVMFlag::Type::Bool});
if (flag != nullptr && flag->get()) {
flag->set(0);
}
#endif // __aarch64__
}

Comment on lines +102 to +111
Copy link
Member

@ivoanjo ivoanjo Oct 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think we should have better documentation here of:

  • Why is this needed (work around JDK 8 bug on ... )
  • What happens if this is not here

"// aarch64 does not support CriticalJNINatives" I'm not sure will be enough for someone 1/2/n years from now to understand why this was added.

Bonus points for reflecting this into the method name e.g. installWorkaroundForJdk8ArmCrash (or something similarly descriptive) would be a good way of conveying some of this information :)

const void *VMStructs_::findHeapUsageFunc() {
if (VM::hotspot_version() < 17) {
// For JDK 11 it is really unreliable to find the memory_usage function -
Expand Down
1 change: 1 addition & 0 deletions ddprof-lib/src/main/cpp/vmStructs_dd.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ namespace ddprof {
static void initOffsets();
static void initJvmFunctions();
static void initUnsafeFunctions();
static void initCriticalJNINatives();

static void checkNativeBinding(jvmtiEnv *jvmti, JNIEnv *jni, jmethodID method,
void *address);
Expand Down
8 changes: 4 additions & 4 deletions ddprof-lib/src/main/java/com/datadoghq/profiler/JavaProfiler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ public void stop() throws IllegalStateException {
*
* @return Number of samples
*/
public native long getSamples();
public static native long getSamples();

/**
* Get profiler agent version, e.g. "1.0"
Expand Down Expand Up @@ -470,10 +470,10 @@ public Map<String, Long> getDebugCounters() {
private native void stop0() throws IllegalStateException;
private native String execute0(String command) throws IllegalArgumentException, IllegalStateException, IOException;

private native void filterThreadAdd0();
private native void filterThreadRemove0();
private static native void filterThreadAdd0();
private static native void filterThreadRemove0();
// Backward compatibility for existing code
private native void filterThread0(boolean enable);
private static native void filterThread0(boolean enable);

private static native int getTid0();
private static native ByteBuffer getContextPage0(int tid);
Expand Down
Loading