-
Notifications
You must be signed in to change notification settings - Fork 11
Add remote symbolication support with build-id and PC offset #324
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
c52da1c
bd80f4c
04a7e9c
161ad44
55fbce3
0f3951b
dde78ee
a98db7a
8604a97
0fa6f7d
6924e55
ff8476f
fd63395
ef0b7e1
d63365e
1b7b1cc
e3d9a43
8d758bf
1215dc1
0e4235c
45d0122
3fa35e2
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 |
|---|---|---|
|
|
@@ -88,7 +88,10 @@ static const Multiplier UNIVERSAL[] = { | |
| // samples | ||
| // generations - track surviving generations | ||
| // lightweight[=BOOL] - enable lightweight profiling - events without | ||
| // stacktraces (default: true) jfr - dump events in Java | ||
| // stacktraces (default: true) | ||
| // remotesym[=BOOL] - enable remote symbolication for native frames | ||
| // (stores build-id and PC offset instead of symbol names) | ||
| // jfr - dump events in Java | ||
| // Flight Recorder format interval=N - sampling interval in ns | ||
| // (default: 10'000'000, i.e. 10 ms) jstackdepth=N - maximum Java stack | ||
| // depth (default: 2048) safemode=BITS - disable stack recovery | ||
|
|
@@ -339,18 +342,30 @@ Error Arguments::parse(const char *args) { | |
| _enable_method_cleanup = true; | ||
| } | ||
|
|
||
| CASE("wallsampler") | ||
| CASE("remotesym") | ||
| if (value != NULL) { | ||
| switch (value[0]) { | ||
| case 'j': | ||
| _wallclock_sampler = JVMTI; | ||
| case 'y': // yes | ||
| case 't': // true | ||
| _remote_symbolication = true; | ||
| break; | ||
| case 'a': | ||
| default: | ||
| _wallclock_sampler = ASGCT; | ||
| _remote_symbolication = false; | ||
| } | ||
| } | ||
|
Comment on lines
+345
to
355
|
||
|
|
||
| CASE("wallsampler") | ||
| if (value != NULL) { | ||
| switch (value[0]) { | ||
| case 'j': | ||
| _wallclock_sampler = JVMTI; | ||
| break; | ||
| case 'a': | ||
| default: | ||
| _wallclock_sampler = ASGCT; | ||
| } | ||
| } | ||
|
|
||
| DEFAULT() | ||
| if (_unknown_arg == NULL) | ||
| _unknown_arg = arg; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -114,6 +114,25 @@ void Lookup::fillNativeMethodInfo(MethodInfo *mi, const char *name, | |
| } | ||
| } | ||
|
|
||
| void Lookup::fillRemoteFrameInfo(MethodInfo *mi, const RemoteFrameInfo *rfi) { | ||
| // Store build-id in the class name field | ||
| mi->_class = _classes->lookup(rfi->build_id); | ||
|
|
||
| // Store PC offset in hex format in the signature field | ||
| char offset_hex[32]; | ||
| snprintf(offset_hex, sizeof(offset_hex), "0x%lx", rfi->pc_offset); | ||
|
Comment on lines
+122
to
+123
|
||
| mi->_sig = _symbols.lookup(offset_hex); | ||
|
|
||
| // Use same modifiers as regular native frames (0x100 = ACC_NATIVE for consistency) | ||
| mi->_modifiers = 0x100; | ||
| // Use FRAME_NATIVE_REMOTE type to indicate remote symbolication | ||
| mi->_type = FRAME_NATIVE_REMOTE; | ||
| mi->_line_number_table = nullptr; | ||
|
|
||
| // Method name indicates need for remote symbolication | ||
| mi->_name = _symbols.lookup("<remote>"); | ||
| } | ||
|
|
||
| void Lookup::cutArguments(char *func) { | ||
| char *p = strrchr(func, ')'); | ||
| if (p == NULL) | ||
|
|
@@ -322,6 +341,9 @@ MethodInfo *Lookup::resolveMethod(ASGCT_CallFrame &frame) { | |
| const char *name = (const char *)method; | ||
| fillNativeMethodInfo(mi, name, | ||
| Profiler::instance()->getLibraryName(name)); | ||
| } else if (frame.bci == BCI_NATIVE_FRAME_REMOTE) { | ||
| const RemoteFrameInfo *rfi = (const RemoteFrameInfo *)method; | ||
| fillRemoteFrameInfo(mi, rfi); | ||
| } else { | ||
| fillJavaMethodInfo(mi, method, first_time); | ||
| } | ||
|
|
@@ -1036,18 +1058,23 @@ void Recording::writeNativeLibraries(Buffer *buf) { | |
| if (_recorded_lib_count < 0) | ||
| return; | ||
|
|
||
| Profiler *profiler = Profiler::instance(); | ||
| CodeCacheArray &native_libs = profiler->_native_libs; | ||
| Libraries *libraries = Libraries::instance(); | ||
| const CodeCacheArray &native_libs = libraries->native_libs(); | ||
| int native_lib_count = native_libs.count(); | ||
|
|
||
| for (int i = _recorded_lib_count; i < native_lib_count; i++) { | ||
| CodeCache* lib = native_libs[i]; | ||
|
|
||
| // Emit jdk.NativeLibrary event with extended fields (buildId and loadBias) | ||
| flushIfNeeded(buf, RECORDING_BUFFER_LIMIT - MAX_STRING_LENGTH); | ||
| int start = buf->skip(5); | ||
| buf->putVar64(T_NATIVE_LIBRARY); | ||
| buf->putVar64(_start_ticks); | ||
| buf->putUtf8(native_libs[i]->name()); | ||
| buf->putVar64((uintptr_t)native_libs[i]->minAddress()); | ||
| buf->putVar64((uintptr_t)native_libs[i]->maxAddress()); | ||
| buf->putUtf8(lib->name()); | ||
| buf->putVar64((uintptr_t)lib->minAddress()); | ||
| buf->putVar64((uintptr_t)lib->maxAddress()); | ||
| buf->putUtf8(lib->hasBuildId() ? lib->buildId() : ""); | ||
| buf->putVar64((uintptr_t)lib->loadBias()); | ||
| buf->putVar32(start, buf->offset() - start); | ||
| flushIfNeeded(buf); | ||
| } | ||
|
|
||
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.
The documentation comment indicates "remotesym[=BOOL]" but the implementation doesn't follow the typical boolean argument pattern. It should either accept standard boolean values (true/false, yes/no, 1/0) or the comment should clarify that only 'y' and 't' are accepted for true. Consider aligning the implementation with the documented interface or updating the documentation to match actual behavior.