On Sun, 15 Oct 2023 20:20:48 GMT, Erik Gahlin <egah...@openjdk.org> wrote:

> Hi,
> 
> Could I have a review of an enhancement that replaces the use of ASM with the 
> new Class-File API. This change only deals with bytecode that writes event 
> data into buffers. Bytecode transformations carried out by classes in 
> jdk.jfr.internal.intrument package are kept as is. Plan is to try to replace 
> those with events in java.base.
> 
> To simplify the review process, I have tried to keep the code in the classes 
> EventInstrumentation and EventClassBuilder similar to what existed before. 
> Further refactorizations may happen at a later stage. 
> 
> Testing: tier1-3 + jdk/jdk/jfr
> 
> Thanks
> Erik

src/jdk.jfr/share/classes/jdk/jfr/internal/EventClassBuilder.java line 73:

> 71:         byte[] bytes = Classfile.of().build(ClassDesc.of(fullClassName), 
> cb -> build(cb));
> 72:         try {
> 73:             String name = "/Users/egahlin/DynamicEvent" + 
> (idCounter.longValue() - 1);

You probably wish to use `Bytecode.log` instead.

src/jdk.jfr/share/classes/jdk/jfr/internal/EventInstrumentation.java line 204:

> 202:         String typeDescriptor = classDesc.descriptorString();
> 203:         for (ClassElement ce : classModel.elements()) {
> 204:             if (ce instanceof RuntimeVisibleAnnotationsAttribute rvaa) {

You can use `classModel.findAttribute(Attributes.RUNTIME_VISIBLE_ANNOTATIONS)` 
instead of looping.

src/jdk.jfr/share/classes/jdk/jfr/internal/EventInstrumentation.java line 227:

> 225:         for (MethodModel m : classModel.methods()) {
> 226:             for (MethodElement me : m.elements()) {
> 227:                 if (me instanceof RuntimeVisibleAnnotationsAttribute 
> rvaa) {

Same remark, `m.findAttribute(Attributes.RUNTIME_VISIBLE_ANNOTATIONS)`

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/16195#discussion_r1359971991
PR Review Comment: https://git.openjdk.org/jdk/pull/16195#discussion_r1359976477
PR Review Comment: https://git.openjdk.org/jdk/pull/16195#discussion_r1359976987

Reply via email to