On Wed, 27 May 2026 14:56:10 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). > > Daisuke Yamazaki has updated the pull request incrementally with three > additional commits since the last revision: > > - Include BSM entry in garbage prefill for constant pool hashing > - Fix hash code computation in BootstrapMethodEntry to use poolHash > - Correct incomplete comment Sorry, never clicked the button to leave the messages. We might need to.distinguish between the lookup and equality hash, as lookup hash can trust same-pool invariants. src/java.base/share/classes/jdk/internal/classfile/impl/AbstractPoolEntry.java line 530: > 528: abstract static sealed class AbstractRefEntry<T extends PoolEntry> > extends AbstractPoolEntry { > 529: protected final T ref1; > 530: private @Stable int contentHash; I called the one in Utf8EntryImpl contentHash because it is a string hash instead of a directly usable hash for an entry. So we need a better name. I think we might use names like `constructionHash` and `fullHash` instead. And the cache field for the full hash might be stored in the root AbstractPoolEntry, which can declare like this: int constructionHash() { // Just return `hashCode()` by default? // The "hash" field should get a rename otherwise. assert hash != 0; return hash; } private @Stable int fullHash; public int hashCode() { int hash = fullHash; if (hash == 0) { return fullHash = computeFullHash(); } return hash; } abstract int computeFullHash(); ------------- PR Review: https://git.openjdk.org/jdk/pull/31255#pullrequestreview-4373731858 PR Review Comment: https://git.openjdk.org/jdk/pull/31255#discussion_r3311922142
