On Thu, 12 Feb 2026 09:23:14 GMT, Paul Hübner <[email protected]> wrote:
>> src/hotspot/share/prims/jvm.cpp line 798: >> >>> 796: // matter when this is called the same identity hash code is >>> expected. >>> 797: // 2. Oops: the above still applies, but the oops' identity hash >>> code must >>> 798: // be used as the polymorphic hashCode may change due to >>> mutability. >> >> This is a good description for generating the identity hashcode but seems >> like its in the wrong place... this function doesn't "own" the calculation >> of the hash and the various cases - they belong elsewhere. >> >> The first line `// The generated identity hash is invariantly immutable.` is >> the only part of this that applies in this function. >> >> The reason this comment is odd here is that we don't implement 2 cases here. > > While this function doesn't own the calculation, my intention with this > comment was to demystify the black-box call that is > `ValueObjectMethods.valueObjectHashCode` in order to justify why the > assertions provided later on are sound. When implementing > [JDK-8376171](https://bugs.openjdk.org/browse/JDK-8376171), the VM and Java > were using slightly different assumptions, leading to > [JDK-8376512](https://bugs.openjdk.org/browse/JDK-8376512). I think that a) > this mistake would have been spotted earlier and b) the review would not have > taken as long if there was some non-surfacelevel documentation in `IHashCode`. > > That said, I do agree that there are more applicable/appropriate places to > document the behaviour. How would you feel about moving this comment to the > JavaDoc of `VOM.valueObjectHashCode` and keeping a shorter summary in this > place? Moving the description of the two cases to VOM.valueObjectHashCode makes sense to me. That's the code that logically owns the cases ------------- PR Review Comment: https://git.openjdk.org/valhalla/pull/2029#discussion_r2799064657
