-
Notifications
You must be signed in to change notification settings - Fork 937
Sample JVM method exit via bytecode rewriting #1421
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
Sample JVM method exit via bytecode rewriting #1421
Conversation
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.
Clang-Tidy found issue(s) with the introduced code (1/1)
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.
Clang-Tidy found issue(s) with the introduced code (1/1)
|
We're also missing |
bb14c68 to
579e2d6
Compare
9a51a88 to
2cbfe2a
Compare
src/instrument.cpp
Outdated
| const char* class_name; | ||
| const char* method_name; | ||
| int out = rewriteClass(&class_name, &method_name); | ||
| if (out == 0) { |
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.
Can be a case in switch below
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.
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.
Actually, I reverted this after 0f9931d, because this would cause some repetition where I handle errors
src/instrument.cpp
Outdated
|
|
||
| constexpr u16 MAX_CODE_LENGTH = 65534; | ||
|
|
||
| enum { |
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.
Make it named enum and let functions return this type instead of int.
Constants no longer need to be negative.
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.
Description
In this PR I enable recording JVM method exit via bytecode instrumentation. The bulk of this PR consists in:
I'm also adding a new parameter called
latency.latencyis absent, the previous behavior is keptlatencyis specified (can be 0) any execution of the method which exceedslatencyis going to record a sample.totalwill contain thedurationof the method execution.Related issues
n/a
Motivation and context
There are several use cases enabled by this PR:
How has this been tested?
New tests in
InstrumentTests.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.