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

Remove unused Method handles from Hibernate instrumentation #3496

Merged
merged 1 commit into from
Apr 13, 2022

Conversation

mcculls
Copy link
Contributor

@mcculls mcculls commented Apr 12, 2022

These jlr.Method values are recreated* on every read and are not used in the decorator
(except for method name which byte-buddy makes available in cached form by using '#m')

Removing them should reduce tracer overhead on every hibernate call.

* see warning in the @Advice.Origin javadoc

Note: A constant representing a Method or Constructor is not cached but is recreated for every read

… in the decorator

(except for method name which byte-buddy makes available in cached form by using '#m')

Removing them should reduce tracer overhead on every hibernate call.

(* see warning in the @Advice.Origin javadoc)
@mcculls mcculls added the inst: others All other instrumentations label Apr 12, 2022
@mcculls mcculls requested a review from a team as a code owner April 12, 2022 20:56
@@ -25,7 +24,6 @@ public class SessionMethodUtils {
public static <TARGET, ENTITY> SessionState startScopeFrom(
final ContextStore<TARGET, SessionState> contextStore,
final TARGET spanKey,
final Method origin,
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if there was intent to use this at some point. Nice catch!

Copy link
Contributor

Choose a reason for hiding this comment

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

From the git history it looks like a leftover from changes to reduce the cost of computing the operationName.

Copy link
Contributor

@bantonsson bantonsson left a comment

Choose a reason for hiding this comment

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

🎉

@@ -25,7 +24,6 @@ public class SessionMethodUtils {
public static <TARGET, ENTITY> SessionState startScopeFrom(
final ContextStore<TARGET, SessionState> contextStore,
final TARGET spanKey,
final Method origin,
Copy link
Contributor

Choose a reason for hiding this comment

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

From the git history it looks like a leftover from changes to reduce the cost of computing the operationName.

@mcculls mcculls merged commit fe95d5e into master Apr 13, 2022
@mcculls mcculls deleted the mcculls/removeUnusedHibernateMethodHandles branch April 13, 2022 11:08
@github-actions github-actions bot added this to the 0.100.0 milestone Apr 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
inst: others All other instrumentations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants