RFR: 8301995: Move invokedynamic resolution information out of ConstantPoolCacheEntry [v4]

[Calvin Cheung]

On Tue, 14 Mar 2023 20:20:41 GMT, Matias Saavedra Silva <matsaave at openjdk.org> wrote:

>> The current structure used to store the resolution information for invokedynamic, ConstantPoolCacheEntry, is difficult to interpret due to its ambigious fields f1 and f2. This structure can hold information for fields, methods, and invokedynamics and each of its fields can hold different types of values depending on the entry. 
>> 
>> This enhancement proposes a new structure to exclusively contain invokedynamic information in a manner that is easy to interpret and easy to extend.  Resolved invokedynamic entries will be stored in an array in the constant pool cache and the operand of the invokedynamic bytecode will be rewritten to be the index into this array.
>> 
>> Any areas that previously accessed invokedynamic data from ConstantPoolCacheEntry will be replaced with accesses to this new array and structure. Verified with tier1-9 tests.
>> 
>> The PPC was provided by @reinrich and the RISCV port was provided by @DingliZhang and @zifeihan.
>> 
>> This change supports the following platforms: x86, aarch64, PPC, and RISCV
>
> Matias Saavedra Silva has updated the pull request incrementally with one additional commit since the last revision:
> 
>   RISCV port update

Looks good. Just a few minor comments.

src/hotspot/share/interpreter/bootstrapInfo.cpp line 218:

> 216:                                                           _indy_index,
> 217:                                                           pool()->tag_at(_bss_index),
> 218:                                                           CHECK_false);

Please indent lines 216-218 like before.

src/hotspot/share/interpreter/bootstrapInfo.cpp line 234:

> 232:   if (_indy_index > -1) {
> 233:     os::snprintf_checked(what, sizeof(what), "indy#%d", _indy_index);
> 234:   }

Since the `else` case doesn’t have braces, maybe omit the braces for this case as well?

src/hotspot/share/oops/cpCache.cpp line 618:

> 616:                  indy_resolution_failed(), parameter_size());
> 617:     if ((bytecode_1() == Bytecodes::_invokehandle)) {
> 618:       constantPoolHandle cph(Thread::current(), cache->constant_pool());

There is another `cph` defined at line 601. Could that one be used?

src/hotspot/share/oops/cpCache.cpp line 652:

> 650:   int size = ConstantPoolCache::size(length);
> 651: 
> 652:   // Initialize resolvedinvokedynamicinfo array with available data

Maybe breakup the long word `resolvedinvokedynamicinfo`?

src/hotspot/share/oops/cpCache.cpp line 653:

> 651: 
> 652:   // Initialize resolvedinvokedynamicinfo array with available data
> 653:   Array<ResolvedIndyEntry>* array;

Suggestion: rename `array` to `resolved_indy_entries`.

src/hotspot/share/oops/cpCache.cpp line 664:

> 662: 
> 663:   return new (loader_data, size, MetaspaceObj::ConstantPoolCacheType, THREAD)
> 664:     ConstantPoolCache(length, index_map, invokedynamic_map, array);

I think it reads better if this line is indented to right after the open parenthesis.

src/hotspot/share/prims/methodComparator.cpp line 119:

> 117:     if ((old_cp->name_ref_at(index_old) != new_cp->name_ref_at(index_new)) ||
> 118:           (old_cp->signature_ref_at(index_old) != new_cp->signature_ref_at(index_new)))
> 119:         return false;

Please adjust the indentations of lines 118 and 119 to be the same as lines 124 and 125.

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/interpreter/BytecodeWithCPIndex.java line 61:

> 59:      } else {
> 60:       return cpCache.getEntryAt((int) (0xFFFF & cpCacheIndex)).getConstantPoolIndex();
> 61:      }

Maybe align all `return` statements with line 56?

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/oops/ResolvedIndyArray.java line 38:

> 36: public class ResolvedIndyArray extends GenericArray {
> 37:   static {
> 38:     VM.registerVMInitializedObserver(new Observer() {

Indentation for java code should be 4 spaces.

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/oops/ResolvedIndyEntry.java line 38:

> 36:   private static long          size;
> 37:   private static long          baseOffset;
> 38:   private static CIntegerField cpIndex;

Indentation for java code should be 4 spaces.

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

PR: https://git.openjdk.org/jdk/pull/12778