On Sun, 15 Oct 2023 20:20:48 GMT, Erik Gahlin <[email protected]> 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
