-
Notifications
You must be signed in to change notification settings - Fork 12
Don't preload jmethodIDs when cstack=vm with hotspot JVM #549
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
5f5fa35
4e3bf98
957d27c
14728a7
64e183b
c782c3c
c462d85
7710c6f
6c6f1a7
07c3a4c
4827c68
3dda4e3
a826e7e
9d1818a
13349c3
06b1a92
b95c88c
df5e992
c26aeea
b2a9a1d
b56c6b4
7567904
79adaea
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,10 +13,11 @@ | |
| #include "context_api.h" | ||
| #include "counters.h" | ||
| #include "dictionary.h" | ||
| #include "flightRecorder.h" | ||
| #include "flightRecorder.inline.h" | ||
| #include "incbin.h" | ||
| #include "jfrMetadata.h" | ||
| #include "jniHelper.h" | ||
| #include "jvmSupport.inline.h" | ||
| #include "os.h" | ||
| #include "profiler.h" | ||
| #include "signalSafety.h" | ||
|
|
@@ -485,8 +486,17 @@ MethodInfo *Lookup::resolveMethod(ASGCT_CallFrame &frame) { | |
| static const char* UNKNOWN = "unknown"; | ||
| unsigned long key; | ||
| jint bci = frame.bci; | ||
| jmethodID method_id = frame.method_id; | ||
|
|
||
| jmethodID method = frame.method_id; | ||
| // Resolve native method | ||
| if (FrameType::isRawPointer(bci)) { | ||
| method_id = JVMSupport::resolve(frame.method); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [MEDIUM / correctness] The raw Please confirm
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's what we do :-) |
||
| } | ||
|
|
||
| // Maps JMETHODID_NOT_WAKEABLE back to nullptr | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [LOW / consistency] Typo: |
||
| if (method_id == JMETHODID_NOT_WALKABLE) { | ||
| method_id = nullptr; | ||
| } | ||
|
|
||
| // BCI_VTABLE_RECEIVER: method holds a VMSymbol* (see vmEntry.h). Resolve | ||
| // to a class_id via the per-dump cache once, then key MethodMap by the | ||
|
|
@@ -495,10 +505,10 @@ MethodInfo *Lookup::resolveMethod(ASGCT_CallFrame &frame) { | |
| // row. | ||
| u32 vtable_class_id = 0; | ||
| if (bci == BCI_VTABLE_RECEIVER) { | ||
| vtable_class_id = resolveVTableReceiverCached((void *)method); | ||
| vtable_class_id = resolveVTableReceiverCached((void *)method_id); | ||
| } | ||
|
|
||
| if (method == nullptr) { | ||
| if (method_id == nullptr) { | ||
| key = MethodMap::makeKey(UNKNOWN); | ||
| } else if (bci == BCI_ERROR || bci == BCI_NATIVE_FRAME) { | ||
| key = MethodMap::makeKey(frame.native_function_name); | ||
|
|
@@ -511,7 +521,7 @@ MethodInfo *Lookup::resolveMethod(ASGCT_CallFrame &frame) { | |
| assert(frame_type == FRAME_INTERPRETED || frame_type == FRAME_JIT_COMPILED || | ||
| frame_type == FRAME_INLINED || frame_type == FRAME_C1_COMPILED || | ||
| VM::isOpenJ9()); // OpenJ9 may have bugs that produce invalid frame types | ||
| key = MethodMap::makeKey(method); | ||
| key = MethodMap::makeKey(method_id); | ||
| } | ||
|
|
||
| MethodInfo *mi = &(*_method_map)[key]; | ||
|
|
@@ -522,12 +532,12 @@ MethodInfo *Lookup::resolveMethod(ASGCT_CallFrame &frame) { | |
| if (first_time) { | ||
| mi->_key = _method_map->size() + 1; // avoid zero key | ||
| } | ||
| if (method == nullptr) { | ||
| if (method_id == nullptr) { | ||
| fillNativeMethodInfo(mi, UNKNOWN, nullptr); | ||
| } else if (bci == BCI_ERROR) { | ||
| fillNativeMethodInfo(mi, (const char *)method, nullptr); | ||
| fillNativeMethodInfo(mi, (const char *)method_id, nullptr); | ||
| } else if (bci == BCI_NATIVE_FRAME) { | ||
| const char *name = (const char *)method; | ||
| const char *name = (const char *)method_id; | ||
| fillNativeMethodInfo(mi, name, | ||
| Profiler::instance()->getLibraryName(name)); | ||
| } else if (bci == BCI_NATIVE_FRAME_REMOTE) { | ||
|
|
@@ -575,7 +585,7 @@ MethodInfo *Lookup::resolveMethod(ASGCT_CallFrame &frame) { | |
| mi->_type = FRAME_NATIVE; | ||
| mi->_is_entry = false; | ||
| } else { | ||
| fillJavaMethodInfo(mi, method, first_time); | ||
| fillJavaMethodInfo(mi, method_id, first_time); | ||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| /* | ||
| * Copyright The async-profiler authors | ||
| * Copyright 2026, Datadog, Inc. | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| #include "flightRecorder.h" | ||
|
|
||
| #include "jvmSupport.inline.h" | ||
|
|
||
|
|
||
| jint MethodInfo::getLineNumber(jint bci) { | ||
| // if the shared pointer is not pointing to the line number table, consider | ||
| // size 0 | ||
| if (!_line_number_table || _line_number_table->_size == 0) { | ||
| return 0; | ||
| } | ||
|
|
||
| int i = 1; | ||
| while (i < _line_number_table->_size && | ||
| bci >= ((jvmtiLineNumberEntry *)_line_number_table->_ptr)[i] | ||
| .start_location) { | ||
| i++; | ||
| } | ||
| return ((jvmtiLineNumberEntry *)_line_number_table->_ptr)[i - 1] | ||
| .line_number; | ||
| } | ||
|
|
||
| bool MethodInfo::isHidden() { | ||
| return JVMSupport::isHidden(_modifiers); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,9 @@ | ||
| #ifndef _FRAME_H | ||
| #define _FRAME_H | ||
|
|
||
| #include <cassert> | ||
| #include "vmEntry.h" | ||
|
|
||
| enum FrameTypeId { | ||
| FRAME_INTERPRETED = 0, | ||
| FRAME_JIT_COMPILED = 1, | ||
|
|
@@ -14,9 +17,11 @@ enum FrameTypeId { | |
| }; | ||
|
|
||
| class FrameType { | ||
| static constexpr int RAW_POINTER_MASK = 1 << 30; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When tiered compilation produces a Useful? React with 👍 / 👎. |
||
| public: | ||
| static inline int encode(int type, int bci) { | ||
| return (1 << 24) | (type << 25) | (bci & 0xffffff); | ||
| static inline int encode(int type, int bci, bool rawPointer = false) { | ||
| assert((!rawPointer || VM::isHotspot()) && "Raw pointer is only valid for hotspot"); | ||
| return (1 << 24) | (type << 25) | (bci & 0xffffff) | (rawPointer ? RAW_POINTER_MASK : 0); | ||
| } | ||
|
|
||
| static inline FrameTypeId decode(int bci) { | ||
|
|
@@ -25,9 +30,13 @@ class FrameType { | |
| return FRAME_JIT_COMPILED; | ||
| } | ||
| // Clamp to valid FrameTypeId range to defend against corrupted values | ||
| int raw_type = bci >> 25; | ||
| int raw_type = (bci & ~ RAW_POINTER_MASK) >> 25; | ||
| return (FrameTypeId)(raw_type <= FRAME_TYPE_MAX ? raw_type : FRAME_TYPE_MAX); | ||
| } | ||
|
|
||
| static inline bool isRawPointer(int bci) { | ||
| return bci > 0 && (bci & RAW_POINTER_MASK) != 0; | ||
| } | ||
| }; | ||
|
|
||
| #endif // _FRAME_H | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With this default set to
false, every normal HotSpot start that does not passfjmethodidgoes throughHotspotSupport::loadMethodIDsImpl()and skips preloading jmethodIDs for bootstrap/platform/application classes. That raw-Method*fallback is only added in the newwalkVMpath; the default ASGCT-based CPU/wall stack collection still depends on preallocated jmethodIDs, so ordinary profiling modes can start returning missing/unknown Java frames for the common system-loader application methods. The field comment also says this should be true by default, so initialize it to true and only disable it for the intendedcstack=vm/explicit opt-out cases.Useful? React with 👍 / 👎.