On Fri, 22 May 2026 10:57:42 GMT, Daisuke Yamazaki <[email protected]> wrote:

> I fixed inconsistent `equals` / `hashCode` behaviour in `PoolEntry` and 
> `BootstrapMethodEntry` objects created by different `ConstantPoolBuilder` 
> instances.
> 
> Previously, some hash codes depended on constant pool indices of referenced 
> entries.
> Because of this, two entries with the same content could be equal but still 
> have different hash codes if they came from different pools.
> 
> This patch splits hashing into two types:
> 1. `public hashCode()`, now based only on content and consistent with 
> `equals`(slower than the type 2)
> 2. internal pool lookup hash, still based on indices for fast 
> `SplitConstantPool` and bootstrap method table deduplication
> 
> This keeps fast local lookup performance while fixing the public `equals` / 
> `hashCode` contract.
> 
> Added `PoolEntryHashCodeTest`, migrated from the original issue reproducer, 
> which compares equivalent entries created by distinct `ConstantPoolBuilders` 
> with different index layouts.
> 
> 
> Running test 'jtreg:test/jdk/jdk/classfile/PoolEntryHashCodeTest.java'
> Passed: jdk/classfile/PoolEntryHashCodeTest.java
> Test results: passed: 1
> 
> 
> ---------
> - [x] I confirm that I make this contribution in accordance with the [OpenJDK 
> Interim AI Policy](https://openjdk.org/legal/ai).

Thanks for working on fixing this.

src/java.base/share/classes/jdk/internal/classfile/impl/BootstrapMethodEntryImpl.java
 line 84:

> 82:             argumentsHash = 31 * argumentsHash + argument.index();
> 83:         }
> 84:         return (31 * handle.index() + argumentsHash) | 
> AbstractPoolEntry.NON_ZERO;

This should probably use `AbstractPoolEntry​::poolHash()`:
Suggestion:

        for (LoadableConstantEntry argument : arguments) {
            argumentsHash = 31 * argumentsHash + ((AbstractPoolEntry) 
argument).poolHash();
        }
        return (31 * handle.poolHash() + argumentsHash) | 
AbstractPoolEntry.NON_ZERO;

test/jdk/jdk/classfile/PoolEntryHashCodeTest.java line 88:

> 86:         ConstantPoolBuilder pool2 = ConstantPoolBuilder.of();
> 87: 
> 88:         // Prefill the pools with some rubbish entries to make

This comment is incomplete:
Suggestion:

        // Prefill the pools with some rubbish entries to offset
        // the indices so that the index-based hash codes differ

test/jdk/jdk/classfile/PoolEntryHashCodeTest.java line 153:

> 151:             pool.utf8Entry("ignore: " + i);
> 152:         }
> 153:     }

My version in the bug report also intentionally added a `BootstrapMethodEntry` 
to this pool.

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

PR Review: https://git.openjdk.org/jdk/pull/31255#pullrequestreview-4372950900
PR Review Comment: https://git.openjdk.org/jdk/pull/31255#discussion_r3311319453
PR Review Comment: https://git.openjdk.org/jdk/pull/31255#discussion_r3311278987
PR Review Comment: https://git.openjdk.org/jdk/pull/31255#discussion_r3311272324

Reply via email to