On Mon, 9 Jun 2025 13:58:09 GMT, Johan Sjölen <jsjo...@openjdk.org> wrote:
>> Hi,
>>
>> The constant pool currently has a lot of methods specific to extracting
>> parts of the operands array. What this array actually is, is a sequence of
>> bootstrap method attribute entries, where each entry has the following
>> components:
>>
>> ```c++
>> struct BSMAE {
>> u2 bootstrap_method_index;
>> u2 argument_count;
>> u2 arguments[argument_count];
>> }
>>
>>
>> We can removes some of these operands array specific methods, and instead
>> allows you to extract BSMAttributeEntrys which you can then use to extract
>> its piece wise components. This makes for a nicer interface, and a bit
>> easier to come into as a reader of the code, as it more closely mirrors the
>> JVMS.
>>
>> Please consider!
>>
>> Testing: Currently GHA, running tier1-tier3
>
> Johan Sjölen has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Move it to public
It is nice update and refactoring in general.
src/hotspot/share/oops/constantPool.cpp line 1953:
> 1951: k1 = bsm_attribute_entry(idx1)->argument_index(j);
> 1952: k2 = cp2->bsm_attribute_entry(idx2)->argument_index(j);
> 1953: match = compare_entry_to(k1, cp2, k2);
Nit: I'd suggest to define two locals to simplify the code as below:
BSMAttributeEntry* e1 = bsm_attribute_entry(idx1);
BSMAttributeEntry& e2 = cp2->bsm_attribute_entry(idx12);
int k1 = e1->bootstrap_method_index();
int k2 = e2->bootstrap_method_index();
bool match = compare_entry_to(k1, cp2, k2);
if (!match) {
return false;
}
int argc = e1->argument_count();
if (argc == e2->argument_count()) {
for (int j = 0; j < argc; j++) {
k1 = e1->argument_index(j);
k2 = e2->argument_index(j);
match = compare_entry_to(k1, cp2, k2);
src/hotspot/share/prims/jvmtiClassFileReconstituter.cpp line 29:
> 27: #include "memory/universe.hpp"
> 28: #include "oops/constantPool.hpp"
> 29: #include "oops/constantPool.inline.hpp"
The line 28 is not needed as we already have line 29.
-------------
PR Review: https://git.openjdk.org/jdk/pull/25298#pullrequestreview-2937554075
PR Review Comment: https://git.openjdk.org/jdk/pull/25298#discussion_r2153510993
PR Review Comment: https://git.openjdk.org/jdk/pull/25298#discussion_r2153513749
