On Tue, 16 Sep 2025 06:22:07 GMT, David Holmes <[email protected]> wrote:
>> Hi,
>>
>> This is a refactoring of the way that we store the Bootstrap method
>> attribute in the ConstantPool class. We used to have a single `Array<u2>`
>> which was divided into a section of `u4` offsets and a section which was the
>> actual data. In this refactoring we make this split more clear, by actually
>> allocating an `Array<u4>` to store the offsets in and an `Array<u2>` to
>> store the data in. These arrays are then put into a `BSMAttributeEntries`
>> class, which allows us to separate out the API from that of the rest of the
>> `ConstantPool`.
>>
>> We had multiple instances of the code knowing the layout of the operands
>> array and using this to do 'clever' ways of copying and inserting data into
>> it. See `ConstantPool::copy_operands` and `ConstantPool::resize_operands`. I
>> felt like we could do things in a simpler way, so I added the
>> `start_/end_extension` protocol and added the `InsertionIterator` for this.
>> See `ClassFileParser::parse_classfile_bootstrap_methods_attribute` for how
>> this works. I put several relevant definitions into the inline file in hopes
>> of encouraging the compiler to optimize these appropriately.
>>
>> For the Java SA code, I had to add a `U4Array` class. I also had to fix the
>> vmstructs definitions, etc.
>>
>> On the whole, while this code is a bit less terse, I think it's a good API
>> improvement and the underlying implementation of splitting up the operands
>> array is also an improvement.
>>
>> Testing: Oracle Tier1-Tier5 has been run succesfully multiple times. Before
>> integration, I will merge with master and run these tiers again.
>
> src/hotspot/share/prims/jvmtiClassFileReconstituter.cpp line 394:
>
>> 392: write_attribute_name_index("BootstrapMethods");
>> 393: u4 length = sizeof(u2) + // num_bootstrap_methods
>> 394: // The rest of it
>
> The comment doesn't add any value here.
The previous code did a manual count of the total amount of bytes required. The
comment is a bit sloppy, I replaced it with "The rest of the data for the
attribute is exactly the u2s in the data array.". The goal is to very
explicitly say "we don't need to count, we've got the information already".
> src/hotspot/share/prims/jvmtiClassFileReconstituter.cpp line 398:
>
>> 396: write_u4(length);
>> 397:
>> 398: int num_bootstrap_methods =
>> cpool()->bsm_entries().number_of_entries();
>
> I'm confused about the seemingly different uses of `num_bootstrap_methods`
> here and in the comment above:
>
> sizeof(u2) + // num_bootstrap_methods
>
> ?
I'm trying to explain to the reader what the `sizeof(u2)` stands for, in this
case it stands for the bytes taken up by the `num_bootstrap_methods` field in
`BootstrapMethods_attribute`.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27198#discussion_r2351227073
PR Review Comment: https://git.openjdk.org/jdk/pull/27198#discussion_r2351221844
