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

Do not use arrays in InstrumentationModule #3057

Conversation

mateuszrzeszutek
Copy link
Member

  • Removed arrays from InstrumentationModule, it now uses lists & maps.
  • Removed generating the muzzleReferences field: MuzzleMatcher maintains a reference to ReferenceMatcher, there's no need to keep the reference to the collection of classrefs anymore.
  • Extracted duplicated code fragments in MuzzleCodeGenerator to private methods.

@mateuszrzeszutek mateuszrzeszutek force-pushed the dont-use-arrays-in-instrumentation-module branch from 31ea56b to 09d9a9b Compare May 21, 2021 12:23
Copy link
Member

@trask trask left a comment

Choose a reason for hiding this comment

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

nice

Comment on lines -76 to +74
return new GenerateMuzzleMethodsAndFields(
classVisitor,
implementationContext.getClassFileVersion().isAtLeast(ClassFileVersion.JAVA_V6));
return new GenerateMuzzleMethodsAndFields(classVisitor);
Copy link
Member

Choose a reason for hiding this comment

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

👍

@@ -47,7 +46,6 @@
class MuzzleCodeGenerator implements AsmVisitorWrapper {
private static final Logger log = LoggerFactory.getLogger(MuzzleCodeGenerator.class);

private static final String MUZZLE_REFERENCES_FIELD_NAME = "muzzleReferences";
Copy link
Member

Choose a reason for hiding this comment

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

👍

Comment on lines -563 to +504
mv.visitTypeInsn(Opcodes.NEW, "java/util/HashMap");
// stack: map
mv.visitInsn(Opcodes.DUP);
// stack: map, map
mv.visitLdcInsn(contextStoreClasses.size());
// stack: map, map, size
mv.visitMethodInsn(Opcodes.INVOKESPECIAL, "java/util/HashMap", "<init>", "(I)V", false);
writeNewMap(mv, contextStoreClasses.size());
Copy link
Member

Choose a reason for hiding this comment

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

👍

// stack: map
mv.visitInsn(Opcodes.DUP);
// stack: map, map
// pass bigger size to avoid resizes; same formula as in e.g. HashSet(Collection)
Copy link
Member

Choose a reason for hiding this comment

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

👍

@mateuszrzeszutek mateuszrzeszutek merged commit 6fb3ec0 into open-telemetry:main May 24, 2021
@mateuszrzeszutek mateuszrzeszutek deleted the dont-use-arrays-in-instrumentation-module branch May 24, 2021 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants