Skip to content

Commit 3d4be3a

Browse files
authored
Merge branch 'main' into jb/tiny_downstream
2 parents 04a448d + 0c9e478 commit 3d4be3a

34 files changed

+1014
-480
lines changed

.github/workflows/approve-trivial.yml

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
name: Auto-Approve Trivial PRs
2+
3+
on:
4+
pull_request_target:
5+
types: [labeled]
6+
7+
permissions:
8+
pull-requests: write
9+
contents: read
10+
11+
jobs:
12+
auto-approve:
13+
if: contains(github.event.pull_request.labels.*.name, 'trivial')
14+
runs-on: ubuntu-latest
15+
steps:
16+
- name: Auto-approve PR if labeled 'trivial'
17+
uses: hmarr/auto-approve-action@v4
18+
with:
19+
github-token: ${{ secrets.GITHUB_TOKEN }}

.github/workflows/test_workflow.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ jobs:
8484
export LIBC=glibc
8585
export SANITIZER=${{ matrix.config }}
8686
87-
./gradlew -PkeepJFRs :ddprof-test:test${{ matrix.config }} --no-daemon --parallel --build-cache --no-watch-fs
87+
./gradlew -PCI -PkeepJFRs :ddprof-test:test${{ matrix.config }} --no-daemon --parallel --build-cache --no-watch-fs
8888
EXIT_CODE=$?
8989
9090
if [ $EXIT_CODE -ne 0 ]; then
@@ -174,7 +174,7 @@ jobs:
174174
}')
175175
export JAVA_VERSION
176176
177-
./gradlew -PkeepJFRs :ddprof-test:test${{ matrix.config }} --no-daemon --parallel --build-cache --no-watch-fs
177+
./gradlew -PCI -PkeepJFRs :ddprof-test:test${{ matrix.config }} --no-daemon --parallel --build-cache --no-watch-fs
178178
EXIT_CODE=$?
179179
180180
if [ $EXIT_CODE -ne 0 ]; then
@@ -274,7 +274,7 @@ jobs:
274274
export LIBC=glibc
275275
export SANITIZER=${{ matrix.config }}
276276
277-
./gradlew -PkeepJFRs :ddprof-test:test${{ matrix.config }} --no-daemon --parallel --build-cache --no-watch-fs
277+
./gradlew -PCI -PkeepJFRs :ddprof-test:test${{ matrix.config }} --no-daemon --parallel --build-cache --no-watch-fs
278278
EXIT_CODE=$?
279279
280280
if [ $EXIT_CODE -ne 0 ]; then

build.gradle

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ plugins {
1717
version = "1.21.0"
1818

1919
apply plugin: "com.dipien.semantic-version"
20-
version = project.hasProperty("ddprof_version") ? project.ddprof_version : version
20+
version = project.findProperty("ddprof_version") ?: version
2121

2222
allprojects {
2323
repositories {

ddprof-lib/gtest/build.gradle

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ def buildNativeLibsTask = tasks.register("buildNativeLibs") {
4242
description = "Build the native libs for the Google Tests"
4343

4444
onlyIf {
45-
hasGtest && !project.hasProperty('skip-native') && os().isLinux()
45+
hasGtest && !project.hasProperty('skip-native') && !project.hasProperty('skip-gtest') && os().isLinux()
4646
}
4747

4848
def srcDir = project(':ddprof-lib').file('src/test/resources/native-libs')
@@ -72,7 +72,7 @@ def buildNativeLibsTask = tasks.register("buildNativeLibs") {
7272

7373
def gtestAll = tasks.register("gtest") {
7474
onlyIf {
75-
hasGtest && !project.hasProperty('skip-tests') && !project.hasProperty('skip-native')
75+
hasGtest && !project.hasProperty('skip-tests') && !project.hasProperty('skip-native') && !project.hasProperty('skip-gtest')
7676
}
7777
group = 'verification'
7878
description = "Run all Google Tests for all build configurations of the library"
@@ -94,6 +94,7 @@ tasks.whenTaskAdded { task ->
9494
def gtestCompileTask = tasks.register("compileGtest${config.name.capitalize()}_${testName}", CppCompile) {
9595
onlyIf {
9696
config.active && hasGtest && !project.hasProperty('skip-tests') && !project.hasProperty('skip-native')
97+
&& !project.hasProperty('skip-gtest')
9798
}
9899
group = 'build'
99100
description = "Compile the Google Test ${testName} for the ${config.name} build of the library"
@@ -151,6 +152,7 @@ tasks.whenTaskAdded { task ->
151152
def gtestLinkTask = tasks.register("linkGtest${config.name.capitalize()}_${testName}", LinkExecutable) {
152153
onlyIf {
153154
config.active && hasGtest && !project.hasProperty('skip-tests') && !project.hasProperty('skip-native')
155+
&& !project.hasProperty('skip-gtest')
154156
}
155157
group = 'build'
156158
description = "Link the Google Test for the ${config.name} build of the library"
@@ -175,6 +177,7 @@ tasks.whenTaskAdded { task ->
175177
def gtestExecuteTask = tasks.register("gtest${config.name.capitalize()}_${testName}", Exec) {
176178
onlyIf {
177179
config.active && hasGtest && !project.hasProperty('skip-tests') && !project.hasProperty('skip-native')
180+
&& !project.hasProperty('skip-gtest')
178181
}
179182
group = 'verification'
180183
description = "Run the Google Test ${testName} for the ${config.name} build of the library"

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

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -277,21 +277,18 @@ Error Arguments::parse(const char *args) {
277277

278278
CASE("cstack")
279279
if (value != NULL) {
280-
switch (value[0]) {
281-
case 'n':
282-
_cstack = CSTACK_NO;
283-
break;
284-
case 'd':
280+
if (strcmp(value, "fp") == 0) {
281+
_cstack = CSTACK_FP;
282+
} else if (strcmp(value, "dwarf") == 0) {
285283
_cstack = CSTACK_DWARF;
286-
break;
287-
case 'l':
284+
} else if (strcmp(value, "lbr") == 0) {
288285
_cstack = CSTACK_LBR;
289-
break;
290-
case 'v':
286+
} else if (strcmp(value, "vm") == 0) {
291287
_cstack = CSTACK_VM;
292-
break;
293-
default:
294-
_cstack = CSTACK_FP;
288+
} else if (strcmp(value, "vmx") == 0) {
289+
_cstack = CSTACK_VMX;
290+
} else {
291+
_cstack = CSTACK_NO;
295292
}
296293
}
297294

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

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -61,12 +61,13 @@ enum Style {
6161
};
6262

6363
enum CStack {
64-
CSTACK_DEFAULT,
65-
CSTACK_NO,
66-
CSTACK_FP,
67-
CSTACK_DWARF,
68-
CSTACK_LBR,
69-
CSTACK_VM
64+
CSTACK_DEFAULT, // use perf_event_open stack if available or Frame Pointer links otherwise
65+
CSTACK_NO, // do not collect native frames
66+
CSTACK_FP, // walk stack using Frame Pointer links
67+
CSTACK_DWARF, // use DWARF unwinding info from .eh_frame section
68+
CSTACK_LBR, // Last Branch Record hardware capability
69+
CSTACK_VM, // unwind using HotSpot VMStructs
70+
CSTACK_VMX // same as CSTACK_VM but with intermediate native frames
7071
};
7172

7273
enum Output { OUTPUT_NONE, OUTPUT_COLLAPSED, OUTPUT_JFR };

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,13 @@ const int DW_REG_FP = 29;
5555
const int DW_REG_SP = 31;
5656
const int DW_REG_PC = 30;
5757
const int EMPTY_FRAME_SIZE = 0;
58-
const int LINKED_FRAME_SIZE = 0;
58+
59+
// aarch64 function prologue looks like this (if frame pointer is used):
60+
// stp x29, x30, [sp, -16]! // Save FP (x29) and LR (x30)
61+
// mov x29, sp // Set FP to SP
62+
// ---
63+
// LINKED_FRAME_SIZE should be 16
64+
const int LINKED_FRAME_SIZE = 2 * DW_STACK_SLOT;
5965

6066
#else
6167

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -740,13 +740,15 @@ void Recording::writeSettings(Buffer *buf, Arguments &args) {
740740
writeBoolSetting(buf, T_HEAP_LIVE_OBJECT, "enabled", args._record_liveness);
741741

742742
writeBoolSetting(buf, T_ACTIVE_RECORDING, "debugSymbols",
743-
VMStructs::hasDebugSymbols());
743+
VMStructs::libjvm()->hasDebugSymbols());
744744
writeBoolSetting(buf, T_ACTIVE_RECORDING, "kernelSymbols",
745745
Symbols::haveKernelSymbols());
746746
writeStringSetting(buf, T_ACTIVE_RECORDING, "cpuEngine",
747747
Profiler::instance()->cpuEngine()->name());
748748
writeStringSetting(buf, T_ACTIVE_RECORDING, "wallEngine",
749749
Profiler::instance()->wallEngine()->name());
750+
writeStringSetting(buf, T_ACTIVE_RECORDING, "cstack",
751+
Profiler::instance()->cstack());
750752
flushIfNeeded(buf);
751753
}
752754

@@ -892,9 +894,9 @@ void Recording::writeJvmInfo(Buffer *buf) {
892894
buf->putVar64(_start_ticks);
893895
buf->putUtf8(jvm_name);
894896
buf->putUtf8(jvm_version);
895-
buf->putUtf8(_jvm_args);
896-
buf->putUtf8(_jvm_flags);
897-
buf->putUtf8(_java_command);
897+
buf->putUtf8(_jvm_args != nullptr ? _jvm_args : "");
898+
buf->putUtf8(_jvm_flags != nullptr ? _jvm_flags : "");
899+
buf->putUtf8(_java_command != nullptr ? _java_command : "");
898900
buf->putVar64(OS::processStartTime());
899901
buf->putVar64(OS::processId());
900902
buf->putVar32(start, buf->offset() - start);

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

Lines changed: 38 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66

77
#include "profiler.h"
88
#include "asyncSampleMutex.h"
9-
#include "common.h"
109
#include "context.h"
1110
#include "counters.h"
1211
#include "ctimer.h"
@@ -278,7 +277,7 @@ int Profiler::getNativeTrace(void *ucontext, ASGCT_CallFrame *frames,
278277
PerfEvents::walkKernel(tid, callchain + native_frames,
279278
MAX_NATIVE_FRAMES - native_frames, java_ctx);
280279
}
281-
if (_cstack == CSTACK_VM) {
280+
if (_cstack >= CSTACK_VM) {
282281
return 0;
283282
} else if (_cstack == CSTACK_DWARF) {
284283
native_frames += StackWalker::walkDwarf(ucontext, callchain + native_frames,
@@ -395,7 +394,7 @@ int Profiler::getJavaTraceAsync(void *ucontext, ASGCT_CallFrame *frames,
395394
if (in_java && java_ctx->sp != 0) {
396395
// skip ahead to the Java frames before calling AGCT
397396
frame.restore((uintptr_t)java_ctx->pc, java_ctx->sp, java_ctx->fp);
398-
} else if (state != 0 && vm_thread->lastJavaSP() == 0) {
397+
} else if (state != 0 && (vm_thread->anchor() == nullptr || vm_thread->anchor()->lastJavaSP() == 0)) {
399398
// we haven't found the top Java frame ourselves, and the lastJavaSP wasn't
400399
// recorded either when not in the Java state, lastJava ucontext will be
401400
// used by AGCT
@@ -474,14 +473,16 @@ int Profiler::getJavaTraceAsync(void *ucontext, ASGCT_CallFrame *frames,
474473
}
475474
} else if (trace.num_frames == ticks_unknown_not_Java &&
476475
!(_safe_mode & LAST_JAVA_PC)) {
477-
uintptr_t &sp = vm_thread->lastJavaSP();
478-
uintptr_t &pc = vm_thread->lastJavaPC();
479-
if (sp != 0 && pc == 0) {
476+
JavaFrameAnchor* anchor = vm_thread->anchor();
477+
uintptr_t sp = anchor->lastJavaSP();
478+
const void* pc = anchor->lastJavaPC();
479+
if (sp != 0 && pc == NULL) {
480480
// We have the last Java frame anchor, but it is not marked as walkable.
481481
// Make it walkable here
482-
pc = ((uintptr_t *)sp)[-1];
482+
pc = ((const void**)sp)[-1];
483+
anchor->setLastJavaPC(pc);
483484

484-
NMethod *m = CodeHeap::findNMethod((const void *)pc);
485+
NMethod *m = CodeHeap::findNMethod(pc);
485486
if (m != NULL) {
486487
// AGCT fails if the last Java frame is a Runtime Stub with an invalid
487488
// _frame_complete_offset. In this case we patch _frame_complete_offset
@@ -491,28 +492,29 @@ int Profiler::getJavaTraceAsync(void *ucontext, ASGCT_CallFrame *frames,
491492
m->setFrameCompleteOffset(0);
492493
}
493494
VM::_asyncGetCallTrace(&trace, max_depth, ucontext);
494-
} else if (_libs->findLibraryByAddress((const void *)pc) != NULL) {
495+
} else if (_libs->findLibraryByAddress(pc) != NULL) {
495496
VM::_asyncGetCallTrace(&trace, max_depth, ucontext);
496497
}
497498

498-
pc = 0;
499+
anchor->setLastJavaPC(nullptr);
499500
}
500501
} else if (trace.num_frames == ticks_not_walkable_not_Java &&
501502
!(_safe_mode & LAST_JAVA_PC)) {
502-
uintptr_t &sp = vm_thread->lastJavaSP();
503-
uintptr_t &pc = vm_thread->lastJavaPC();
504-
if (sp != 0 && pc != 0) {
503+
JavaFrameAnchor* anchor = vm_thread->anchor();
504+
uintptr_t sp = anchor->lastJavaSP();
505+
const void* pc = anchor->lastJavaPC();
506+
if (sp != 0 && pc != NULL) {
505507
// Similar to the above: last Java frame is set,
506508
// but points to a Runtime Stub with an invalid _frame_complete_offset
507-
NMethod *m = CodeHeap::findNMethod((const void *)pc);
509+
NMethod *m = CodeHeap::findNMethod(pc);
508510
if (m != NULL && !m->isNMethod() && m->frameSize() > 0 &&
509511
m->frameCompleteOffset() == -1) {
510512
m->setFrameCompleteOffset(0);
511513
VM::_asyncGetCallTrace(&trace, max_depth, ucontext);
512514
}
513515
}
514516
} else if (trace.num_frames == ticks_GC_active && !(_safe_mode & GC_TRACES)) {
515-
if (vm_thread->lastJavaSP() == 0) {
517+
if (vm_thread->anchor()->lastJavaSP() == 0) {
516518
// Do not add 'GC_active' for threads with no Java frames, e.g. Compiler
517519
// threads
518520
frame.restore(saved_pc, saved_sp, saved_fp);
@@ -671,28 +673,32 @@ void Profiler::recordSample(void *ucontext, u64 counter, int tid,
671673
ASGCT_CallFrame *native_stop = frames + num_frames;
672674
num_frames += getNativeTrace(ucontext, native_stop, event_type, tid,
673675
&java_ctx, &truncated);
674-
if (_cstack == CSTACK_VM) {
675-
num_frames +=
676-
StackWalker::walkVM(ucontext, frames + num_frames, _max_stack_depth,
677-
_call_stub_begin, _call_stub_end);
676+
if (_cstack == CSTACK_VMX) {
677+
num_frames += StackWalker::walkVM(ucontext, frames + num_frames, _max_stack_depth, VM_EXPERT, &truncated);
678678
} else if (event_type == BCI_CPU || event_type == BCI_WALL) {
679-
int java_frames = 0;
680-
{
679+
if (_cstack == CSTACK_VM) {
680+
num_frames += StackWalker::walkVM(ucontext, frames + num_frames, _max_stack_depth, VM_NORMAL, &truncated);
681+
} else {
681682
// Async events
682683
AsyncSampleMutex mutex(ProfiledThread::current());
684+
int java_frames = 0;
683685
if (mutex.acquired()) {
684-
java_frames =
685-
getJavaTraceAsync(ucontext, frames + num_frames, _max_stack_depth,
686-
&java_ctx, &truncated);
686+
java_frames = getJavaTraceAsync(ucontext, frames + num_frames, _max_stack_depth, &java_ctx, &truncated);
687+
if (java_frames > 0 && java_ctx.pc != NULL && VMStructs::hasMethodStructs()) {
688+
NMethod* nmethod = CodeHeap::findNMethod(java_ctx.pc);
689+
if (nmethod != NULL) {
690+
fillFrameTypes(frames + num_frames, java_frames, nmethod);
691+
}
692+
}
687693
}
688-
}
689-
if (java_frames > 0 && java_ctx.pc != NULL) {
690-
NMethod *nmethod = CodeHeap::findNMethod(java_ctx.pc);
691-
if (nmethod != NULL) {
692-
fillFrameTypes(frames + num_frames, java_frames, nmethod);
694+
if (java_frames > 0 && java_ctx.pc != NULL) {
695+
NMethod *nmethod = CodeHeap::findNMethod(java_ctx.pc);
696+
if (nmethod != NULL) {
697+
fillFrameTypes(frames + num_frames, java_frames, nmethod);
698+
}
693699
}
700+
num_frames += java_frames;
694701
}
695-
num_frames += java_frames;
696702
}
697703

698704
if (num_frames == 0) {
@@ -1058,7 +1064,7 @@ Error Profiler::checkJvmCapabilities() {
10581064
}
10591065
}
10601066

1061-
if (!VMStructs::hasDebugSymbols() && !VM::isOpenJ9()) {
1067+
if (!VMStructs::libjvm()->hasDebugSymbols() && !VM::isOpenJ9()) {
10621068
Log::warn("Install JVM debug symbols to improve profile accuracy");
10631069
}
10641070

@@ -1151,7 +1157,6 @@ Error Profiler::start(Arguments &args, bool reset) {
11511157
return Error(
11521158
"VMStructs stack walking is not supported on this JVM/platform");
11531159
}
1154-
Log::info("cstack=vm is an experimental option, use with care");
11551160
}
11561161

11571162
// Kernel symbols are useful only for perf_events without --all-user
@@ -1281,7 +1286,7 @@ Error Profiler::check(Arguments &args) {
12811286
return Error("DWARF unwinding is not supported on this platform");
12821287
} else if (args._cstack == CSTACK_LBR && _cpu_engine != &perf_events) {
12831288
return Error("Branch stack is supported only with PMU events");
1284-
} else if (args._cstack == CSTACK_VM && !VMStructs::hasStackStructs()) {
1289+
} else if (_cstack >= CSTACK_VM && !(VMStructs::hasStackStructs() && OS::isLinux())) {
12851290
return Error(
12861291
"VMStructs stack walking is not supported on this JVM/platform");
12871292
}

0 commit comments

Comments
 (0)