RFR: 8320530: has_resolved_ref_index flag not restored after resetting entry [v2]

[David Holmes]

On Tue, 28 Nov 2023 22:22:22 GMT, Matias Saavedra Silva <matsaave at openjdk.org> wrote:

>> ResolvedMethodEntry::reset_entry() clears the fields in the structure and then restores the constant pool index and the resolved references index if it has one. Currently, the resolved references index is restored without restoring the has_resolved_reference flag used by the structure. 
>> 
>> This patch restored the flag with the resolved references index. Verified with tier 1-5 tests.
>
> Matias Saavedra Silva has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains four additional commits since the last revision:
> 
>  - Added asserts to ensure correctness
>  - Merge branch 'master' into resolved_ref_flag
>  - Merge branch 'master' of https://github.com/openjdk/jdk into resolved_ref_flag
>  - 8320530: has_resolved_ref_index flag not restored after resetting entry

A few nits with the `DEBUG_ONLY` code.

src/hotspot/share/oops/resolvedMethodEntry.hpp line 80:

> 78:   u1 _flags;                         // Flags: [00|has_resolved_ref_index|has_local_signature|has_appendix|forced_virtual|final|virtual_final]
> 79:   u1 _bytecode1, _bytecode2;         // Resolved invoke codes
> 80:   DEBUG_ONLY(

Nit: it is better to use `#ifdef ASSERT` than a multi-line `DEBUG_ONLY()` - and that will also allow the correct indentation for the new variables.

src/hotspot/share/oops/resolvedMethodEntry.hpp line 96:

> 94:       _bytecode2(0) {
> 95:         _entry_specific._interface_klass = nullptr;
> 96:         DEBUG_ONLY(

Ditto - use `ifdef ASSERT`

src/hotspot/share/oops/resolvedMethodEntry.hpp line 194:

> 192: 
> 193:   void set_klass(InstanceKlass* klass) {
> 194:     DEBUG_ONLY(

You don't  need this to guard an actual assert statement.

src/hotspot/share/oops/resolvedMethodEntry.hpp line 204:

> 202: 
> 203:   void set_resolved_references_index(u2 ref_index) {
> 204:     DEBUG_ONLY(

You don't need this to guard an actual assert statement.

src/hotspot/share/oops/resolvedMethodEntry.hpp line 214:

> 212: 
> 213:   void set_table_index(u2 table_index) {
> 214:     DEBUG_ONLY(

You don't need this to guard an actual assert statement.

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

Changes requested by dholmes (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16769#pullrequestreview-1754536538
PR Review Comment: https://git.openjdk.org/jdk/pull/16769#discussion_r1408807310
PR Review Comment: https://git.openjdk.org/jdk/pull/16769#discussion_r1408808145
PR Review Comment: https://git.openjdk.org/jdk/pull/16769#discussion_r1408809189
PR Review Comment: https://git.openjdk.org/jdk/pull/16769#discussion_r1408810238
PR Review Comment: https://git.openjdk.org/jdk/pull/16769#discussion_r1408810513