Print string contents for "this" references #20933
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@TobiAjila Could you please take a look? |
h3110n3rv3
commented
Jan 14, 2025
tajila
reviewed
Jan 16, 2025
h3110n3rv3
force-pushed
the
print-string-improvements
branch
2 times, most recently
from
34fd8b7 to
ca5dd4d
Compare
h3110n3rv3
changed the title
tajila
reviewed
Jan 21, 2025
|
@keithc-ca Please review these changes |
keithc-ca
requested changes
Jan 21, 2025
runtime/rastrace/method_trace.c
Outdated
Comment on lines
231
to
235
| sprintf(outputString, "(String)<Memory allocation error>"); | ||
| } else if (utf8Length > maxStringLength) { | ||
| sprintf(outputString, "(String)\"%.*s\"...", (U_32)maxStringLength, utf8String); | ||
| } else { | ||
| sprintf(outputString, "(String)\"%.*s\"", (U_32)utf8Length, utf8String); |
There was a problem hiding this comment.
These should include @%p - like lines 536-540.
There was a problem hiding this comment.
Line 237 takes the receiver as an argument to print the address, that's why it's not added here.
Would you like me to change the format to do this way instead?
h3110n3rv3
force-pushed
the
print-string-improvements
branch
4 times, most recently
from
fa774b7 to
6d6bb46
Compare
The changes reflect the feature request eclipse-openj9#20878. Print string contents for this references. Also, print address for string arguments and returns. Closes: eclipse-openj9#20878 Signed-off-by: Nick Kamal <[email protected]>
h3110n3rv3
force-pushed
the
print-string-improvements
branch
from
26dfab8 to
ff2e56f
Compare
keithc-ca
requested changes
Jan 24, 2025
| TraceEntry=Trc_MethodEntry Overhead=1 Level=5 Group=bytecodeMethods Template="%.*s.%.*s%.*s bytecode method, this = 0x%zx" | ||
| TraceEntry=Trc_MethodEntry Obsolete Overhead=1 Level=5 Group=bytecodeMethods Template="%.*s.%.*s%.*s bytecode method, this = 0x%zx," | ||
|
|
||
| TraceEntry=Trc_MethodEntry Overhead=1 Level=5 Group=bytecodeMethods Template="%.*s.%.*s%.*s bytecode method, this = 0x%zx %.*s" |
There was a problem hiding this comment.
New tracepoints must be added at the end of files like this (and there's no need to change the template of one that is being obsoleted). See https://github.com/eclipse-openj9/openj9/blob/master/doc/diagnostics/AddingTracepoints.md.
| @@ -200,12 +200,48 @@ traceMethodEnter(J9VMThread *thr, J9Method *method, void *receiverAddress, UDATA | |||
| if (modifiers & J9AccNative) { | |||
| Trc_MethodEntryN(thr, J9UTF8_LENGTH(className), J9UTF8_DATA(className), J9UTF8_LENGTH(methodName), J9UTF8_DATA(methodName), J9UTF8_LENGTH(methodSignature), J9UTF8_DATA(methodSignature), receiver); | |||
| } else { | |||
| Trc_MethodEntry(thr, J9UTF8_LENGTH(className), J9UTF8_DATA(className), J9UTF8_LENGTH(methodName), J9UTF8_DATA(methodName), J9UTF8_LENGTH(methodSignature), J9UTF8_DATA(methodSignature), receiver); | |||
| Trc_MethodEntry(thr, J9UTF8_LENGTH(className), J9UTF8_DATA(className), J9UTF8_LENGTH(methodName), J9UTF8_DATA(methodName), J9UTF8_LENGTH(methodSignature), J9UTF8_DATA(methodSignature), receiver, 0, ""); | |||
There was a problem hiding this comment.
Just one space after each comma, please.
| @@ -479,7 +515,7 @@ traceMethodArgObject(J9VMThread *thr, UDATA* arg0EA, char* cursor, UDATA length) | |||
| const unsigned int maxStringLength = RAS_GLOBAL_FROM_JAVAVM(maxStringLength, vm); | |||
|
|
|||
| if (clazz == J9VMJAVALANGSTRING_OR_NULL(vm) | |||
| && (0 != maxStringLength) | |||
| && (0 != maxStringLength) | |||
There was a problem hiding this comment.
Please don't add a leading space here.
| sprintf(outputString, "- (String)\"%.*s\"", (U_32)utf8Length, utf8String); | ||
| } | ||
|
|
||
| Trc_MethodEntry(thr, J9UTF8_LENGTH(className), J9UTF8_DATA(className), J9UTF8_LENGTH(methodName), J9UTF8_DATA(methodName), J9UTF8_LENGTH(methodSignature), J9UTF8_DATA(methodSignature), receiver, J9UTF8_LENGTH(outputString), outputString); |
There was a problem hiding this comment.
Just one space after each comma, please.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The changes reflect the feature request #20878.
Print string contents for
thisreferences.Also, print address for string arguments and returns.
Closes: #20878
Signed-off-by: Nick Kamal <[email protected]>