RFR: 8367656: Refactor Constantpool's operand array into two
Johan Sjölen
jsjolen at openjdk.org
Tue Sep 16 08:08:30 UTC 2025
On Tue, 16 Sep 2025 06:03:09 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/oops/constantPool.cpp line 2348:
>
>> 2346: assert(num_entries + iter._cur_offset <= iter.insert_into->_offsets->length(), "must");
>> 2347: for (int i = 0; i < num_entries; i++) {
>> 2348: const BSMAttributeEntry* bsmae = entry(i);
>
> Nit: It's okay to use a simple name like `e` to represent `entry` - when you don't have different types of entries involved we don't need to encode the type in the variable name. EDIT: just like in `BSMAttributeEntries::InsertionIterator::reserve_new_entry`.
In this particular case, the `entry` method will be shadowed so you have to explicitly write `this->entry()` if you want to use that name. The existence of that variable is so short, we can just call it `e`.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27198#discussion_r2351351291
More information about the serviceability-dev
mailing list