RFR: 8367656: Refactor Constantpool's operand array into two [v7]

[Serguei Spitsyn]

On Mon, 22 Sep 2025 08:02:57 GMT, Johan Sjölen <jsjolen 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.
>
> Johan Sjölen has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Merge remote-tracking branch 'origin/operands-again' into operands-again
>  - Fix BSM naming

src/hotspot/share/oops/constantPool.hpp line 116:

> 114:     return _argument_count;
> 115:   }
> 116:   int argument(int n) const {

Q: Why was the function name changed?

src/hotspot/share/oops/constantPool.hpp line 139:

> 137:     int _cur_offset;
> 138:     // Current unused offset into BSMAEs bsm-data array
> 139:     int _cur_array;

Nit: I'd suggest to rename this to something like `_cur_bsm_entry`.

src/hotspot/share/prims/jvmtiRedefineClasses.hpp line 440:

> 438:   int append_bsm_entry(const constantPoolHandle& scratch_cp, int scratch_bootstrap_spec_index,
> 439:     constantPoolHandle *merge_cp_p, int *merge_cp_length_p);
> 440:   void finalize_bsmentries_merge(const constantPoolHandle& merge_cp, TRAPS);

Nit: A suggestion to rename: `finalize_bsmentries_merge` => `finalize_bsm_entries_merge`

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/oops/ConstantPool.java line 95:

> 93:     Type bsmaetype = db.lookupType("BSMAttributeEntries");
> 94:     bsmaentries_offsets = bsmaetype.getAddressField("_offsets");
> 95:     bsmaentries_bootstrap_methods = bsmaetype.getAddressField("_bootstrap_methods");

Nit: A suggestion to rename:
  `bsmaentries` => `bsma_entries` or `bsm_entries`
  `bsmaetype`   => `bsma_type` or `bsm_type`
  `bsmaentries_offsets` => `bsma_entries_offsets` or `bsm_entries_offsets`
  `bsmaentries_bootstrap_methods` => `bsma_entries_bootstrap_methods` or `bsm_entries_bootstrap_methods`

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/oops/ConstantPool.java line 122:

> 120:   private static long bsmaentries; // Offset in the constantpool where the BSMAEntries are found
> 121:   private static AddressField bsmaentries_offsets;
> 122:   private static AddressField bsmaentries_bootstrap_methods;

Nit: Same renaming suggestion as above.

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/oops/ConstantPool.java line 482:

> 480:     int basePos = offs.at(bsmIndex);
> 481:     int argv = basePos + INDY_ARGV_OFFSET;
> 482:     int argc = getBootstrapMethodArgsCount(bsmIndex);

Nit: Consider to make it shorter:
  `getBootstrapMethods` => `getBSMs`
  `getBootstrapMethodArgsCount` => `getBSMArgsCount`

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27198#discussion_r2371074352
PR Review Comment: https://git.openjdk.org/jdk/pull/27198#discussion_r2371075707
PR Review Comment: https://git.openjdk.org/jdk/pull/27198#discussion_r2371078749
PR Review Comment: https://git.openjdk.org/jdk/pull/27198#discussion_r2371084318
PR Review Comment: https://git.openjdk.org/jdk/pull/27198#discussion_r2371084929
PR Review Comment: https://git.openjdk.org/jdk/pull/27198#discussion_r2371087738