RFR: 8367656: Refactor Constantpool's operand array into two

[Johan Sjölen]

On Tue, 16 Sep 2025 06:22:07 GMT, David Holmes <dholmes at openjdk.org> 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