Skip to content

Commit 934f110

Browse files
zhengyu123jbachorikrkennkeCopilot
authored
Don't preload jmethodIDs when cstack=vm with hotspot JVM (#549)
Co-authored-by: Jaroslav Bachorik <jaroslav.bachorik@datadoghq.com> Co-authored-by: Roman Kennke <roman.kennke@datadoghq.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
1 parent 6f4cbb0 commit 934f110

21 files changed

Lines changed: 1025 additions & 173 deletions

ddprof-lib/src/main/cpp/arguments.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,17 @@ Error Arguments::parse(const char *args) {
253253
msg = "jstackdepth must be > 0";
254254
}
255255

256+
CASE("fjmethodid")
257+
if (value != nullptr) {
258+
if (strcmp(value, "false") == 0) {
259+
_force_jmethodID = false;
260+
} else if (strcmp(value, "true") == 0) {
261+
_force_jmethodID = true;
262+
} else {
263+
msg = "Invalid jmethodID creation value";
264+
}
265+
}
266+
256267
CASE("safemode")
257268
_safe_mode = value == NULL ? INT_MAX : (int)strtol(value, NULL, 0);
258269

ddprof-lib/src/main/cpp/arguments.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,7 @@ class Arguments {
196196
bool _jvmtistacks; // Delegate CPU/wall stack walks to HotSpot JFR RequestStackTrace extension
197197
bool _nativesocket;
198198
long _nativesocket_interval; // initial sampling period in nanoseconds; 0 = engine default
199+
bool _force_jmethodID; // Load all jmethodIDs, true by default
199200

200201
Arguments(bool persistent = false)
201202
: _buf(NULL),
@@ -235,7 +236,8 @@ class Arguments {
235236
_remote_symbolication(false),
236237
_jvmtistacks(false),
237238
_nativesocket(false),
238-
_nativesocket_interval(0) {}
239+
_nativesocket_interval(0),
240+
_force_jmethodID(true) {}
239241

240242
~Arguments();
241243

ddprof-lib/src/main/cpp/flightRecorder.cpp

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,11 @@
1313
#include "context_api.h"
1414
#include "counters.h"
1515
#include "dictionary.h"
16-
#include "flightRecorder.h"
16+
#include "flightRecorder.inline.h"
1717
#include "incbin.h"
1818
#include "jfrMetadata.h"
1919
#include "jniHelper.h"
20+
#include "jvmSupport.inline.h"
2021
#include "os.h"
2122
#include "profiler.h"
2223
#include "signalSafety.h"
@@ -494,8 +495,12 @@ MethodInfo *Lookup::resolveMethod(ASGCT_CallFrame &frame) {
494495
static const char* UNKNOWN = "unknown";
495496
unsigned long key;
496497
jint bci = frame.bci;
498+
jmethodID method_id = frame.method_id;
497499

498-
jmethodID method = frame.method_id;
500+
// Resolve native method
501+
if (FrameType::isRawPointer(bci)) {
502+
method_id = JVMSupport::resolve(frame.method);
503+
}
499504

500505
// BCI_VTABLE_RECEIVER: method holds a VMSymbol* (see vmEntry.h). Resolve
501506
// to a class_id via the per-dump cache once, then key MethodMap by the
@@ -504,10 +509,10 @@ MethodInfo *Lookup::resolveMethod(ASGCT_CallFrame &frame) {
504509
// row.
505510
u32 vtable_class_id = 0;
506511
if (bci == BCI_VTABLE_RECEIVER) {
507-
vtable_class_id = resolveVTableReceiverCached((void *)method);
512+
vtable_class_id = resolveVTableReceiverCached((void *)method_id);
508513
}
509514

510-
if (method == nullptr) {
515+
if (method_id == nullptr) {
511516
key = MethodMap::makeKey(UNKNOWN);
512517
} else if (bci == BCI_ERROR || bci == BCI_NATIVE_FRAME) {
513518
key = MethodMap::makeKey(frame.native_function_name);
@@ -520,7 +525,7 @@ MethodInfo *Lookup::resolveMethod(ASGCT_CallFrame &frame) {
520525
assert(frame_type == FRAME_INTERPRETED || frame_type == FRAME_JIT_COMPILED ||
521526
frame_type == FRAME_INLINED || frame_type == FRAME_C1_COMPILED ||
522527
VM::isOpenJ9()); // OpenJ9 may have bugs that produce invalid frame types
523-
key = MethodMap::makeKey(method);
528+
key = MethodMap::makeKey(method_id);
524529
}
525530

526531
MethodInfo *mi = &(*_method_map)[key];
@@ -536,12 +541,12 @@ MethodInfo *Lookup::resolveMethod(ASGCT_CallFrame &frame) {
536541
// (PROF-15130). The allocator recycles ids freed on erase instead.
537542
mi->_key = _method_map->allocId();
538543
}
539-
if (method == nullptr) {
544+
if (method_id == nullptr) {
540545
fillNativeMethodInfo(mi, UNKNOWN, nullptr);
541546
} else if (bci == BCI_ERROR) {
542-
fillNativeMethodInfo(mi, (const char *)method, nullptr);
547+
fillNativeMethodInfo(mi, (const char *)method_id, nullptr);
543548
} else if (bci == BCI_NATIVE_FRAME) {
544-
const char *name = (const char *)method;
549+
const char *name = (const char *)method_id;
545550
fillNativeMethodInfo(mi, name,
546551
Profiler::instance()->getLibraryName(name));
547552
} else if (bci == BCI_NATIVE_FRAME_REMOTE) {
@@ -589,7 +594,7 @@ MethodInfo *Lookup::resolveMethod(ASGCT_CallFrame &frame) {
589594
mi->_type = FRAME_NATIVE;
590595
mi->_is_entry = false;
591596
} else {
592-
fillJavaMethodInfo(mi, method, first_time);
597+
fillJavaMethodInfo(mi, method_id, first_time);
593598
}
594599
}
595600

@@ -1574,7 +1579,7 @@ int Recording::writeStackTraces(Buffer *buf, Lookup *lookup) {
15741579
jint bci = trace->frames[i].bci;
15751580
if (mi->_type < FRAME_NATIVE) {
15761581
FrameTypeId type = FrameType::decode(bci);
1577-
bci = (bci & 0x10000) ? 0 : (bci & 0xffff);
1582+
bci = FrameType::bci(bci);
15781583
buf->putVar32(mi->getLineNumber(bci));
15791584
buf->putVar32(bci);
15801585
buf->put8(type);

ddprof-lib/src/main/cpp/flightRecorder.h

Lines changed: 2 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -91,27 +91,8 @@ class MethodInfo {
9191
std::shared_ptr<SharedLineNumberTable> _line_number_table;
9292
FrameTypeId _type;
9393

94-
jint getLineNumber(jint bci) {
95-
// if the shared pointer is not pointing to the line number table, consider
96-
// size 0
97-
if (!_line_number_table || _line_number_table->_size == 0) {
98-
return 0;
99-
}
100-
101-
int i = 1;
102-
while (i < _line_number_table->_size &&
103-
bci >= ((jvmtiLineNumberEntry *)_line_number_table->_ptr)[i]
104-
.start_location) {
105-
i++;
106-
}
107-
return ((jvmtiLineNumberEntry *)_line_number_table->_ptr)[i - 1]
108-
.line_number;
109-
}
110-
111-
bool isHidden() {
112-
// 0x1400 = ACC_SYNTHETIC(0x1000) | ACC_BRIDGE(0x0040)
113-
return _modifiers == 0 || (_modifiers & 0x1040);
114-
}
94+
inline jint getLineNumber(jint bci);
95+
inline bool isHidden();
11596
};
11697

11798
// MethodMap's key can be derived from 3 sources:
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
/*
2+
* Copyright The async-profiler authors
3+
* Copyright 2026, Datadog, Inc.
4+
* SPDX-License-Identifier: Apache-2.0
5+
*/
6+
7+
#ifndef _FLIGHTRECORDER_INLINE_H
8+
#define _FLIGHTRECORDER_INLINE_H
9+
10+
11+
#include "flightRecorder.h"
12+
#include "jvmSupport.inline.h"
13+
14+
jint MethodInfo::getLineNumber(jint bci) {
15+
// if the shared pointer is not pointing to the line number table, consider
16+
// size 0
17+
if (!_line_number_table || _line_number_table->_size == 0) {
18+
return 0;
19+
}
20+
21+
int i = 1;
22+
while (i < _line_number_table->_size &&
23+
bci >= ((jvmtiLineNumberEntry *)_line_number_table->_ptr)[i]
24+
.start_location) {
25+
i++;
26+
}
27+
return ((jvmtiLineNumberEntry *)_line_number_table->_ptr)[i - 1]
28+
.line_number;
29+
}
30+
31+
bool MethodInfo::isHidden() {
32+
return JVMSupport::isHidden(_modifiers);
33+
}
34+
35+
#endif // _FLIGHTRECORDER_INLINE_H

ddprof-lib/src/main/cpp/frame.h

Lines changed: 62 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,24 @@
1+
/*
2+
* Copyright 2026 Datadog, Inc
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
116
#ifndef _FRAME_H
217
#define _FRAME_H
318

19+
#include <cassert>
20+
#include "vmEntry.h"
21+
422
enum FrameTypeId {
523
FRAME_INTERPRETED = 0,
624
FRAME_JIT_COMPILED = 1,
@@ -10,23 +28,59 @@ enum FrameTypeId {
1028
FRAME_KERNEL = 5,
1129
FRAME_C1_COMPILED = 6,
1230
FRAME_NATIVE_REMOTE = 7, // Native frame with remote symbolication (build-id + pc-offset)
13-
FRAME_TYPE_MAX = FRAME_NATIVE_REMOTE // Maximum valid frame type
31+
FRAME_TYPE_MAX = FRAME_NATIVE_REMOTE, // Maximum valid frame type
32+
FRAME_TYPE_MASK = 0x7
1433
};
1534

35+
// Packs frame type and BCI into a single int field stored in CallTrace frames.
36+
//
37+
// Bit layout of an encoded value (ENCODED_MASK set):
38+
// bit 30 RAW_POINTER_MASK — value is a raw native PC (HotSpot only)
39+
// bits 23–21 frame type — FrameTypeId (0–7)
40+
// bit 20 ENCODED_MASK — set to distinguish encoded values from raw ASGCT BCIs
41+
// bits 15–0 BCI — bytecode index (0–65534; never 65535, see BCI_MASK)
42+
//
43+
// When ENCODED_MASK is not set the field is a raw, unencoded BCI from ASGCT (non-VM stack
44+
// walking modes). Raw values may be negative: HotSpot uses -1 as a sentinel for method-entry
45+
// and synchronization-entry samples. Both encode() and bci() clamp negative values to 0 so
46+
// that 65535 (the mask value itself) is never emitted and can be treated as unreachable.
47+
// decode() returns FRAME_JIT_COMPILED for all unencoded or negative values.
1648
class FrameType {
49+
// JVM spec §4.7.3 caps method bytecode at 65535 bytes, so valid BCIs are 0–65534.
50+
// The mask value 65535 (0xffff) is therefore never a valid BCI; we keep it unreachable
51+
// by clamping negative sentinels to 0 in encode() and bci().
52+
static constexpr int BCI_MASK = 0xffff;
53+
static constexpr int TYPE_SHIFT = 21;
54+
static constexpr int ENCODED_MASK = 1 << 20; // distinguishes encoded values from raw ASGCT BCIs
55+
static constexpr int RAW_POINTER_MASK = 1 << 30;
1756
public:
18-
static inline int encode(int type, int bci) {
19-
return (1 << 24) | (type << 25) | (bci & 0xffffff);
57+
// Produces an encoded int from a frame type and BCI. Negative BCIs (HotSpot -1 sentinels
58+
// for method-entry/sync-entry) are mapped to 0 rather than wrapping to 0xffff.
59+
static inline int encode(int type, int bci, bool rawPointer = false) {
60+
assert((!rawPointer || VM::isHotspot()) && "Raw pointer is only valid for hotspot");
61+
assert(type >= FRAME_INTERPRETED && type <= FRAME_TYPE_MAX);
62+
int bci_bits = (bci < 0) ? 0 : (bci & BCI_MASK);
63+
return ENCODED_MASK | (type << TYPE_SHIFT) | bci_bits | (rawPointer ? RAW_POINTER_MASK : 0);
64+
}
65+
66+
// Extracts the BCI from either an encoded value or a raw ASGCT BCI.
67+
// Negative values (HotSpot -1 sentinels) are clamped to 0, matching encode().
68+
static inline int bci(int bci) {
69+
return (bci < 0) ? 0 : (bci & BCI_MASK);
2070
}
2171

72+
// Extracts the FrameTypeId from an encoded value.
73+
// Returns FRAME_JIT_COMPILED for unencoded raw ASGCT BCIs and for negative sentinels,
74+
// since no type information is available in those cases.
2275
static inline FrameTypeId decode(int bci) {
23-
if ((bci >> 24) <= 0) {
24-
// Unencoded BCI (bit 24 not set) or negative special BCI values
76+
if ((bci & ENCODED_MASK) == 0 || bci < 0) {
2577
return FRAME_JIT_COMPILED;
2678
}
27-
// Clamp to valid FrameTypeId range to defend against corrupted values
28-
int raw_type = bci >> 25;
29-
return (FrameTypeId)(raw_type <= FRAME_TYPE_MAX ? raw_type : FRAME_TYPE_MAX);
79+
return (FrameTypeId)((bci >> TYPE_SHIFT) & FRAME_TYPE_MASK);
80+
}
81+
82+
static inline bool isRawPointer(int bci) {
83+
return bci > 0 && (bci & RAW_POINTER_MASK) != 0;
3084
}
3185
};
3286

0 commit comments

Comments
 (0)