Skip to content
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

Add support for double values for metric probes #5457

Merged
merged 1 commit into from
Jun 25, 2023

Conversation

jpbempel
Copy link
Member

for gauge and histogram metric types

What Does This Do

Motivation

Additional Notes

for gauge and histogram metric types
@jpbempel jpbempel added the comp: debugger Dynamic Instrumentation label Jun 23, 2023
@jpbempel jpbempel requested a review from a team as a code owner June 23, 2023 09:32
@jpbempel jpbempel requested review from shatzi and removed request for a team June 23, 2023 09:32
@pr-commenter
Copy link

pr-commenter bot commented Jun 23, 2023

Benchmarks

Parameters

Baseline Candidate
commit 1.17.0-SNAPSHOT~e028fb17f9 1.17.0-SNAPSHOT~ae73429d8d
config baseline candidate
See matching parameters
Baseline Candidate
module Agent Agent
parent None None

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 22 cases.

Copy link
Contributor

@shatzi shatzi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

private void convertIfRequired(Type currentType, Type expectedType, InsnList insnList) {
if (expectedType == Type.LONG_TYPE && currentType == Type.INT_TYPE) {
private Type convertIfRequired(Type currentType, InsnList insnList) {
if (currentType == Type.INT_TYPE) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit, if someone define a byte variable - does the byte code store those as int? I wonder if byte type are supported by default.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's relevant question. The thing is, the JVM on the operand stack treats byte short & char as int, see https://docs.oracle.com/javase/specs/jvms/se20/html/jvms-2.html#jvms-2.11.1

Note that most instructions in Table 2.11.1-A do not have forms for the integral types byte, char, and short. None have forms for the boolean type. A compiler encodes loads of literal values of types byte and short using Java Virtual Machine instructions that sign-extend those values to values of type int at compile-time or run-time. Loads of literal values of types boolean and char are encoded using instructions that zero-extend the literal to a value of type int at compile-time or run-time. Likewise, loads from arrays of values of type boolean, byte, short, and char are encoded using Java Virtual Machine instructions that sign-extend or zero-extend the values to values of type int. Thus, most operations on values of actual types boolean, byte, char, and short are correctly performed by instructions operating on values of computational type int.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though, will add in a next PR some unit tests with those type because the internal representation with EL may not match this.

@jpbempel jpbempel merged commit 4a701d0 into master Jun 25, 2023
@jpbempel jpbempel deleted the jpbempel/double-metric-values branch June 25, 2023 09:26
@github-actions github-actions bot added this to the 1.17.0 milestone Jun 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp: debugger Dynamic Instrumentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants