Skip to content

Don't preload jmethodIDs when cstack=vm with hotspot JVM#549

Open
zhengyu123 wants to merge 40 commits into
mainfrom
hs_jmethod_id
Open

Don't preload jmethodIDs when cstack=vm with hotspot JVM#549
zhengyu123 wants to merge 40 commits into
mainfrom
hs_jmethod_id

Conversation

@zhengyu123

@zhengyu123 zhengyu123 commented May 27, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?:
Reduce memory overhead by storing raw Method pointers instead of pre-allocating jmethodIDs for classes loaded by system class loaders (bootstrap, platform and application)

This feature only applies to hotspot JVM when cstack=vm

Motivation:
Pre-allocating jmethodIDs can consume significant amount of native memory, e.g. 9G in some extreme cases.
By introducing optional lazy jmethodID resolution for system classes, HotSpot JVM profiling avoids loading jmethodIDs for every method, but only the methods on stacks, a significant optimization for native memory usage.

Additional Notes:
This feature is optional and is off by default. To enable the feature, set force_jmethodID=false

How to test the change?:
Running renaissance akka benchmark with NMT, exams jmethodID sites:

Mainline (without this PR)

[0x0000726344c5f43d] InstanceKlass::get_jmethod_id(methodHandle const&)+0x8d
[0x000072634500e78a] Method::jmethod_id()+0x6a
[0x0000726344e78f63] JvmtiEnv::GetClassMethods(oopDesc*, int*, _jmethodID***)+0x2b3
[0x0000726344e2eb86] jvmti_GetClassMethods+0x206
                             (malloc=430672 type=Class #3156) (at peak)

[0x000072634500d4d1] Method::ensure_jmethod_ids(ClassLoaderData*, int)+0xd1
[0x0000726344e78f57] JvmtiEnv::GetClassMethods(oopDesc*, int*, _jmethodID***)+0x2a7
[0x0000726344e2eb86] jvmti_GetClassMethods+0x206
[0x0000726344145374] _jvmtiEnv::GetClassMethods(_jclass*, int*, _jmethodID***)+0x34
                             (malloc=392776 type=Internal #2397) (at peak)

[0x000072634500d48d] Method::ensure_jmethod_ids(ClassLoaderData*, int)+0x8d
[0x0000726344e78f57] JvmtiEnv::GetClassMethods(oopDesc*, int*, _jmethodID***)+0x2a7
[0x0000726344e2eb86] jvmti_GetClassMethods+0x206
[0x0000726344145374] _jvmtiEnv::GetClassMethods(_jclass*, int*, _jmethodID***)+0x34
                             (malloc=57528 type=Class #2397) (at peak)

With force_jmethodID=false

[0x0000762891b93651] Method::ensure_jmethod_ids(ClassLoaderData*, int)+0x251
[0x00007628919fef57] JvmtiEnv::GetClassMethods(oopDesc*, int*, _jmethodID***)+0x2a7
[0x00007628919b4b86] jvmti_GetClassMethods+0x206
[0x0000762890ce9594] _jvmtiEnv::GetClassMethods(_jclass*, int*, _jmethodID***)+0x34
                             (malloc=3920 type=Class #98) (at peak)

[0x0000762891b93850] Method::make_jmethod_id(ClassLoaderData*, Method*)+0x90
[0x00007628917e548d] InstanceKlass::get_jmethod_id(methodHandle const&)+0xdd
[0x0000762891b9478a] Method::jmethod_id()+0x6a
[0x00007628918a0186] get_method_id(JNIEnv_*, _jclass*, char const*, char const*, bool, JavaThread*) [clone .constprop.0]+0x186
                             (malloc=1536 type=Class #64) (at peak)

[0x00007628917e543d] InstanceKlass::get_jmethod_id(methodHandle const&)+0x8d
[0x0000762891b9478a] Method::jmethod_id()+0x6a
[0x00007628919fef63] JvmtiEnv::GetClassMethods(oopDesc*, int*, _jmethodID***)+0x2b3
[0x00007628919b4b86] jvmti_GetClassMethods+0x206
                             (malloc=252656 type=Class #1792) (at peak)

For Datadog employees:

  • If this PR touches code that signs or publishes builds or packages, or handles
    credentials of any kind, I've requested a review from @DataDog/security-design-and-guidance.
  • This PR doesn't touch any of that.
  • JIRA: PROF-13548

Unsure? Have a question? Request a review!

@datadog-official

This comment has been minimized.

@dd-octo-sts

dd-octo-sts Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

CI Test Results

Run: #27226908405 | Commit: 40505c9 | Duration: 12m 39s (longest job)

All 32 test jobs passed

Status Overview

JDK glibc-aarch64/debug glibc-amd64/debug musl-aarch64/debug musl-amd64/debug
8 - - -
8-ibm - - -
8-j9 - -
8-librca - -
8-orcl - - -
11 - - -
11-j9 - -
11-librca - -
17 - -
17-graal - -
17-j9 - -
17-librca - -
21 - -
21-graal - -
21-librca - -
25 - -
25-graal - -
25-librca - -

Legend: ✅ passed | ❌ failed | ⚪ skipped | 🚫 cancelled

Summary: Total: 32 | Passed: 32 | Failed: 0


Updated: 2026-06-09 18:58:57 UTC

@dd-octo-sts

dd-octo-sts Bot commented May 31, 2026

Copy link
Copy Markdown
Contributor

Scan-Build Report

User:runner@runnervm3jyl0
Working Directory:/home/runner/work/java-profiler/java-profiler/ddprof-lib/src/test/make
Command Line:make -j4 all
Clang Version:Ubuntu clang version 18.1.3 (1ubuntu1)
Date:Tue Jun 2 20:41:00 2026

Bug Summary

Bug TypeQuantityDisplay?
All Bugs1
Logic error
Result of operation is garbage or undefined1

Reports

Bug Group Bug Type ▾ File Function/Method Line Path Length
Logic errorResult of operation is garbage or undefinedhotspotSupport.cpploadMethodIDsImpl13092

@zhengyu123 zhengyu123 marked this pull request as ready for review June 2, 2026 15:26
@zhengyu123 zhengyu123 requested a review from a team as a code owner June 2, 2026 15:26

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 13349c38df

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp
Comment thread ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp Outdated
@zhengyu123 zhengyu123 marked this pull request as draft June 2, 2026 15:45
@zhengyu123 zhengyu123 changed the title WIP: Don't preload jmethodIDs when cstack=vm Don't preload jmethodIDs when cstack=vm with hotspot JVM Jun 2, 2026
@zhengyu123 zhengyu123 marked this pull request as ready for review June 2, 2026 17:13

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b95c88c66f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp
Comment thread ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: df5e992559

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp Outdated
@zhengyu123 zhengyu123 requested a review from jbachorik June 2, 2026 20:03

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 39149aff27

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ed1c0471bb

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread ddprof-lib/src/main/cpp/jvmSupport.inline.h
Comment thread ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp Outdated

@rkennke rkennke left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for making the changes. Some old comments are still un-addressed, and I also have some new comments.

Comment thread ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp Outdated
Comment thread ddprof-lib/src/main/cpp/hotspot/vmStructs.h
Comment thread ddprof-lib/src/main/cpp/flightRecorder.cpp Outdated
Comment thread ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp
Comment thread ddprof-lib/src/main/cpp/jvmSupport.inline.h Outdated
Comment thread ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp Outdated
Comment thread ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp
Comment thread ddprof-lib/src/main/cpp/flightRecorder.inline.h Outdated
Comment thread ddprof-lib/src/main/cpp/hotspot/vmStructs.h Outdated
}

// This method only resolves methods that are loaded by system class loaders
jmethodID HotspotSupport::resolve(const void* method) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This must only ever be called outside a signal-handler-context. If possible, this should be asserted, or otherwise at least documented.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added comment in .h file.

@rkennke rkennke Jun 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Where? I don't see it.

In header file:

    // Resolve a method to a jmethodID at dumping time
    static jmethodID resolve(const void* method);

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b5a898f44d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread ddprof-lib/src/main/cpp/jvmSupport.cpp Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: aa92b9bfde

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread ddprof-lib/src/main/cpp/jvmSupport.cpp Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: aa92b9bfde

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread ddprof-lib/src/main/cpp/hotspot/hotspotSupport.h

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b8a3e094fa

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread ddprof-lib/src/main/cpp/jvmSupport.cpp

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 396fec6935

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +1300 to +1301
// Prepare JVMSupport for execution
JVMSupport::initExecution(VM::jvmti(), VM::jni());

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Initialize jmethodID policy after cstack fallback

When a start request uses cstack=vm on a HotSpot where VM stack walking is not supported, this call computes the partial-preload policy before _cstack is normalized and later downgraded to DWARF/no at lines 1317-1337. The recording then samples through the ASGCT path, which still needs preallocated jmethodIDs, but loadAllMethodIDsIfNeeded() has already skipped normal system/application classes, so Java frames in that fallback recording can be serialized as unknown; move this initialization until after the final cstack mode is known or pass the resolved mode into the policy.

Useful? React with 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bb03bac35d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread ddprof-lib/src/main/cpp/jvmSupport.cpp Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5b99ea1960

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp Outdated
Comment thread ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp Outdated
}

// This method only resolves methods that are loaded by system class loaders
jmethodID HotspotSupport::resolve(const void* method) {

@rkennke rkennke Jun 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Where? I don't see it.

In header file:

    // Resolve a method to a jmethodID at dumping time
    static jmethodID resolve(const void* method);

Comment thread ddprof-lib/src/main/cpp/jvmSupport.inline.h Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b41f1b337c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread ddprof-lib/src/main/cpp/thread.cpp Outdated
This reverts commit b41f1b3.

@rkennke rkennke left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Some things you said you fixed are still not fixed, and I also found a new thing.

Comment thread ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp Outdated
Comment thread ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp Outdated
Comment thread ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp Outdated
Comment thread ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 370e84d0f8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread ddprof-lib/src/main/cpp/arguments.h Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0154c29ef8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +42 to +45
static constexpr jint JVM_ACC_HIDDEN = 0x800;
static constexpr jint hidden_mask = (ACC_SYNTHETIC | ACC_BRIDGE | JVM_ACC_HIDDEN);
return modifiers != 0 && // JVMTI GetClassModifiers returns 0 for ordinary package-private classes
((modifiers & hidden_mask) != 0);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Detect hidden classes with the real HotSpot flag

Fresh evidence in this revision is that the replacement helper still keys hidden-class detection off 0x800/synthetic/bridge bits. In the partial-preload path this is the only guard before system/application-loader classes skip GetClassMethods, but arbitrary Lookup.defineHiddenClass classes are unloadable and need not be synthetic or lambda-shaped; those classes can therefore be sampled as raw Method* frames and unload before JFR serialization, leaving the resolver with stale VM metadata and producing missing/unknown Java frames.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants