Re: RFR: JDK-8274686 : java.util.UUID#hashCode() should use Long.hashCode()
On Mon, 4 Oct 2021 21:13:02 GMT, PROgrm_JARvis wrote: > This is trivial fix of > [JDK-8274686](https://bugs.java.com/bugdatabase/view_bug.do?bug_id=JDK-8274686) > which replaces manually-computed `int`-based `long` hash-code. > > Because `Long#hashCode(long)` uses other hashing function than the one > currently used here: > > https://github.com/openjdk/jdk/blob/75d6688df9845ecb8f370b4cd2d5a36f13d3cdc0/src/java.base/share/classes/java/lang/Long.java#L1440-L1442 > > the value of `hashCode()` will produce different result, however this does > not seem to be a breaking change as `UUID#hashCode()` is not specified. > > --- > > Note: the comment to the bug states: > >> Moved to JDK for further discussions of the proposed enhancement. > > But as there seemed to be no corresponding discussion in core-libs-dev and > the change looks trivial, I considered that it is appropriate to open a PR > which (if needed) may be used for discussion (especially, considering the > fact that its comments get mirrored to the ML). Hi there, just a friendly reminder on this PR (as @bridgekeeper has attempted to mark it as stale). Is there anything what should be done in order to have this PR integrated or is it just waiting for its time? - PR: https://git.openjdk.java.net/jdk/pull/5811
Re: RFR: JDK-8274686 : java.util.UUID#hashCode() should use Long.hashCode()
On Wed, 6 Oct 2021 09:26:06 GMT, Peter Levart wrote: >> This is trivial fix of >> [JDK-8274686](https://bugs.java.com/bugdatabase/view_bug.do?bug_id=JDK-8274686) >> which replaces manually-computed `int`-based `long` hash-code. >> >> Because `Long#hashCode(long)` uses other hashing function than the one >> currently used here: >> >> https://github.com/openjdk/jdk/blob/75d6688df9845ecb8f370b4cd2d5a36f13d3cdc0/src/java.base/share/classes/java/lang/Long.java#L1440-L1442 >> >> the value of `hashCode()` will produce different result, however this does >> not seem to be a breaking change as `UUID#hashCode()` is not specified. >> >> --- >> >> Note: the comment to the bug states: >> >>> Moved to JDK for further discussions of the proposed enhancement. >> >> But as there seemed to be no corresponding discussion in core-libs-dev and >> the change looks trivial, I considered that it is appropriate to open a PR >> which (if needed) may be used for discussion (especially, considering the >> fact that its comments get mirrored to the ML). > > Yes, the reverted change to BitSet.hashCode in > https://github.com/openjdk/jdk/pull/4309 has the same property. It could be > applied without any visible effect. @plevart should I then file a follow-up ticket for `BitSet`? And should we change the JavaDoc providing there we have computation formula explicitly specified? - PR: https://git.openjdk.java.net/jdk/pull/5811
Re: RFR: JDK-8274686 : java.util.UUID#hashCode() should use Long.hashCode()
On Mon, 4 Oct 2021 21:13:02 GMT, PROgrm_JARvis wrote: > This is trivial fix of > [JDK-8274686](https://bugs.java.com/bugdatabase/view_bug.do?bug_id=JDK-8274686) > which replaces manually-computed `int`-based `long` hash-code. > > Because `Long#hashCode(long)` uses other hashing function than the one > currently used here: > > https://github.com/openjdk/jdk/blob/75d6688df9845ecb8f370b4cd2d5a36f13d3cdc0/src/java.base/share/classes/java/lang/Long.java#L1440-L1442 > > the value of `hashCode()` will produce different result, however this does > not seem to be a breaking change as `UUID#hashCode()` is not specified. > > --- > > Note: the comment to the bug states: > >> Moved to JDK for further discussions of the proposed enhancement. > > But as there seemed to be no corresponding discussion in core-libs-dev and > the change looks trivial, I considered that it is appropriate to open a PR > which (if needed) may be used for discussion (especially, considering the > fact that its comments get mirrored to the ML). Yes, the reverted change to BitSet.hashCode in https://github.com/openjdk/jdk/pull/4309 has the same property. It could be applied without any visible effect. - PR: https://git.openjdk.java.net/jdk/pull/5811
Re: RFR: JDK-8274686 : java.util.UUID#hashCode() should use Long.hashCode()
On Mon, 4 Oct 2021 21:13:02 GMT, PROgrm_JARvis wrote: > This is trivial fix of > [JDK-8274686](https://bugs.java.com/bugdatabase/view_bug.do?bug_id=JDK-8274686) > which replaces manually-computed `int`-based `long` hash-code. > > Because `Long#hashCode(long)` uses other hashing function than the one > currently used here: > > https://github.com/openjdk/jdk/blob/75d6688df9845ecb8f370b4cd2d5a36f13d3cdc0/src/java.base/share/classes/java/lang/Long.java#L1440-L1442 > > the value of `hashCode()` will produce different result, however this does > not seem to be a breaking change as `UUID#hashCode()` is not specified. > > --- > > Note: the comment to the bug states: > >> Moved to JDK for further discussions of the proposed enhancement. > > But as there seemed to be no corresponding discussion in core-libs-dev and > the change looks trivial, I considered that it is appropriate to open a PR > which (if needed) may be used for discussion (especially, considering the > fact that its comments get mirrored to the ML). Sorry, I was mislead by the comment above that because the original hashCode function is different, the hashCode will be different too. But I think this is not the case here. Original function is this: long hilo = mostSigBits ^ leastSigBits; return ((int)(hilo >> 32)) ^ (int) hilo; new function (with inlined Long.hashCode(long)) looks like this: long hilo = mostSigBits ^ leastSigBits; return (int)(hilo ^ hilo >>> 32); The difference is two-fold: 1. original function uses arithmetic shift right (division by 2^32 preserving the sign) while new function uses logical shift right (shifting in zeros from the left to the right) 2. original function casts individual arguments to int before doing XOR between them while new function XORs long arguments and applies cast to int at the end. But those two differences do not affect the lower 32 bits of any of the intermediate results and therefore the end result is the same (unless I missed something). So I think no release notes is needed for this change. I think that https://github.com/openjdk/jdk/pull/4309 also dealt with similar change which was then reverted as a consequence (I'll have to check to be sure). - PR: https://git.openjdk.java.net/jdk/pull/5811
Re: RFR: JDK-8274686 : java.util.UUID#hashCode() should use Long.hashCode()
On Wed, 6 Oct 2021 08:33:21 GMT, Peter Levart wrote: >> This is trivial fix of >> [JDK-8274686](https://bugs.java.com/bugdatabase/view_bug.do?bug_id=JDK-8274686) >> which replaces manually-computed `int`-based `long` hash-code. >> >> Because `Long#hashCode(long)` uses other hashing function than the one >> currently used here: >> >> https://github.com/openjdk/jdk/blob/75d6688df9845ecb8f370b4cd2d5a36f13d3cdc0/src/java.base/share/classes/java/lang/Long.java#L1440-L1442 >> >> the value of `hashCode()` will produce different result, however this does >> not seem to be a breaking change as `UUID#hashCode()` is not specified. >> >> --- >> >> Note: the comment to the bug states: >> >>> Moved to JDK for further discussions of the proposed enhancement. >> >> But as there seemed to be no corresponding discussion in core-libs-dev and >> the change looks trivial, I considered that it is appropriate to open a PR >> which (if needed) may be used for discussion (especially, considering the >> fact that its comments get mirrored to the ML). > > You do know that this might break things? If there are multiple versions of > JDK present in some distributed system where participants do not agree on the > hash code of an UUID value, it can behave erratically. For example using UUID > as a key in a distributed cache like Infinispan is known to be troublesome if > the hashCode of some key is not the same across the cluster. Usually there > will not be a problem since all nodes in a cluster would use the same JDK > version, but what about a rolling upgrade then? It would not be possible. I > think at least this change needs to be documented in release notes. @plevart may have a point and since this patch doesn't really have any benefits then maybe this PR/issue can be closed. - PR: https://git.openjdk.java.net/jdk/pull/5811
Re: RFR: JDK-8274686 : java.util.UUID#hashCode() should use Long.hashCode()
On Mon, 4 Oct 2021 21:13:02 GMT, PROgrm_JARvis wrote: > This is trivial fix of > [JDK-8274686](https://bugs.java.com/bugdatabase/view_bug.do?bug_id=JDK-8274686) > which replaces manually-computed `int`-based `long` hash-code. > > Because `Long#hashCode(long)` uses other hashing function than the one > currently used here: > > https://github.com/openjdk/jdk/blob/75d6688df9845ecb8f370b4cd2d5a36f13d3cdc0/src/java.base/share/classes/java/lang/Long.java#L1440-L1442 > > the value of `hashCode()` will produce different result, however this does > not seem to be a breaking change as `UUID#hashCode()` is not specified. > > --- > > Note: the comment to the bug states: > >> Moved to JDK for further discussions of the proposed enhancement. > > But as there seemed to be no corresponding discussion in core-libs-dev and > the change looks trivial, I considered that it is appropriate to open a PR > which (if needed) may be used for discussion (especially, considering the > fact that its comments get mirrored to the ML). You do know that this might break things? If there are multiple versions of JDK present in some distributed system where participants do not agree on the hash code of an UUID value, it can behave erratically. For example using UUID as a key in a distributed cache like Infinispan is known to be troublesome if the hashCode of some key is not the same across the cluster. Usually there will not be a problem since all nodes in a cluster would use the same JDK version, but what about a rolling upgrade then? It would not be possible. I think at least this change needs to be documented in release notes. - PR: https://git.openjdk.java.net/jdk/pull/5811
Re: RFR: JDK-8274686 : java.util.UUID#hashCode() should use Long.hashCode()
On Mon, 4 Oct 2021 21:13:02 GMT, PROgrm_JARvis wrote: > This is trivial fix of > [JDK-8274686](https://bugs.java.com/bugdatabase/view_bug.do?bug_id=JDK-8274686) > which replaces manually-computed `int`-based `long` hash-code. > > Because `Long#hashCode(long)` uses other hashing function than the one > currently used here: > > https://github.com/openjdk/jdk/blob/75d6688df9845ecb8f370b4cd2d5a36f13d3cdc0/src/java.base/share/classes/java/lang/Long.java#L1440-L1442 > > the value of `hashCode()` will produce different result, however this does > not seem to be a breaking change as `UUID#hashCode()` is not specified. > > --- > > Note: the comment to the bug states: > >> Moved to JDK for further discussions of the proposed enhancement. > > But as there seemed to be no corresponding discussion in core-libs-dev and > the change looks trivial, I considered that it is appropriate to open a PR > which (if needed) may be used for discussion (especially, considering the > fact that its comments get mirrored to the ML). Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5811
Re: RFR: JDK-8274686 : java.util.UUID#hashCode() should use Long.hashCode()
On Mon, 4 Oct 2021 21:13:02 GMT, PROgrm_JARvis wrote: > This is trivial fix of > [JDK-8274686](https://bugs.java.com/bugdatabase/view_bug.do?bug_id=JDK-8274686) > which replaces manually-computed `int`-based `long` hash-code. > > Because `Long#hashCode(long)` uses other hashing function than the one > currently used here: > > https://github.com/openjdk/jdk/blob/75d6688df9845ecb8f370b4cd2d5a36f13d3cdc0/src/java.base/share/classes/java/lang/Long.java#L1440-L1442 > > the value of `hashCode()` will produce different result, however this does > not seem to be a breaking change as `UUID#hashCode()` is not specified. > > --- > > Note: the comment to the bug states: > >> Moved to JDK for further discussions of the proposed enhancement. > > But as there seemed to be no corresponding discussion in core-libs-dev and > the change looks trivial, I considered that it is appropriate to open a PR > which (if needed) may be used for discussion (especially, considering the > fact that its comments get mirrored to the ML). Marked as reviewed by stsypa...@github.com (no known OpenJDK username). - PR: https://git.openjdk.java.net/jdk/pull/5811
Re: RFR: JDK-8274686 : java.util.UUID#hashCode() should use Long.hashCode()
On Mon, 4 Oct 2021 21:13:02 GMT, PROgrm_JARvis wrote: > This is trivial fix of > [JDK-8274686](https://bugs.java.com/bugdatabase/view_bug.do?bug_id=JDK-8274686) > which replaces manually-computed `int`-based `long` hash-code. > > Because `Long#hashCode(long)` uses other hashing function than the one > currently used here: > > https://github.com/openjdk/jdk/blob/75d6688df9845ecb8f370b4cd2d5a36f13d3cdc0/src/java.base/share/classes/java/lang/Long.java#L1440-L1442 > > the value of `hashCode()` will produce different result, however this does > not seem to be a breaking change as `UUID#hashCode()` is not specified. > > --- > > Note: the comment to the bug states: > >> Moved to JDK for further discussions of the proposed enhancement. > > But as there seemed to be no corresponding discussion in core-libs-dev and > the change looks trivial, I considered that it is appropriate to open a PR > which (if needed) may be used for discussion (especially, considering the > fact that its comments get mirrored to the ML). Good catch, this is what I've missed in https://github.com/openjdk/jdk/pull/4309 - PR: https://git.openjdk.java.net/jdk/pull/5811
RFR: JDK-8274686 : java.util.UUID#hashCode() should use Long.hashCode()
This is trivial fix of [JDK-8274686](https://bugs.java.com/bugdatabase/view_bug.do?bug_id=JDK-8274686) which replaces manually-computed `int`-based `long` hash-code. Because `Long#hashCode(long)` uses other hashing function than the one currently used here: https://github.com/openjdk/jdk/blob/75d6688df9845ecb8f370b4cd2d5a36f13d3cdc0/src/java.base/share/classes/java/lang/Long.java#L1440-L1442 the value of `hashCode()` will produce different result, however this does not seem to be a breaking change as `UUID#hashCode()` is not specified. --- Note: the comment to the bug states: > Moved to JDK for further discussions of the proposed enhancement. But as I there seemed to be no corresponding discussion in core-libs-dev and the change looks trivial, I considered that it is appropriate to open a PR which (if needed) may be used for discussion (especially, considering the fact that its comments get mirrored to the ML). - Commit messages: - fix: use `Long#hashCode(long)` for `UUID#hashCode()` Changes: https://git.openjdk.java.net/jdk/pull/5811/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5811=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8274686 Stats: 2 lines in 1 file changed: 0 ins; 1 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/5811.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5811/head:pull/5811 PR: https://git.openjdk.java.net/jdk/pull/5811
