RFR: 8311102: Write annotations in the classfile dumped by SA

Chris Plummer cjplummer at openjdk.org
Tue Jul 11 05:25:13 UTC 2023


On Fri, 30 Jun 2023 14:32:03 GMT, Ashutosh Mehra <duke at openjdk.org> wrote:

> Please review this PR that enables ClassWriter to write annotations to the class file being dumped.
> 
> The fields annotations are stored in `Annotations::_fields_annotations` which is of type `Array<Array<u1>*>`. There is no class in SA that can represent it. I have added ArrayOfU1Array to correspond to the type `Array<Array<u1>*>` and it works. I believe there are better approaches but that would require a bit more refactoring of the classes representing Array types. I will leave that for future work for now.
> 
> Testing: `test/hotspot/jtreg/serviceability/sa` and `test/jdk/sun/tools/jhsdb`
> Tested it manually by dumping j.l.String class and comparing the annotations with the original class using javap.
> The test case mentioned in [JDK-8311101](https://bugs.openjdk.org/browse/JDK-8311101) would provide better coverage.

Overall the changes look good. Just a few minor suggestions.

src/hotspot/share/runtime/vmStructs.cpp line 972:

> 970:   unchecked_nonstatic_field(Array<Klass*>,            _data,                                  sizeof(Klass*))                        \
> 971:   unchecked_nonstatic_field(Array<ResolvedIndyEntry>, _data,                                  sizeof(ResolvedIndyEntry))             \
> 972:   unchecked_nonstatic_field(Array<Array<u1>*>,         _data,                                 sizeof(Array<u1>*))                    \

Fix alignment of the _data column.

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/oops/Annotations.java line 74:

> 72:   public U1Array getFieldAnnotations(int fieldIndex) {
> 73:     Address addr = fieldsAnnotations.getValue(getAddress());
> 74:     ArrayOfU1Array annotationsArray = VMObjectFactory.newObject(ArrayOfU1Array.class, addr);

How about caching this result so you don't need to allocate a new object every time this API is called. Same thing in `getFieldTypeAnnotations()`.

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/oops/ConstMethod.java line 451:

> 449:         offset += 1;
> 450:       }
> 451:       Address addr = getAddress().getAddressAt((getSize() - offset) * VM.getVM().getAddressSize());

A comment on the address computation would be useful here and in the changes below.

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

Changes requested by cjplummer (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/14735#pullrequestreview-1507639071
PR Review Comment: https://git.openjdk.org/jdk/pull/14735#discussion_r1248137517
PR Review Comment: https://git.openjdk.org/jdk/pull/14735#discussion_r1259190786
PR Review Comment: https://git.openjdk.org/jdk/pull/14735#discussion_r1259199965


More information about the serviceability-dev mailing list