RFR: 8357220: Introduce a BSMAttributeEntry struct [v4]

[Serguei Spitsyn]

On Mon, 9 Jun 2025 13:58:09 GMT, Johan Sjölen <jsjolen at 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