RFR: 8311188: Simplify and modernize equals and hashCode in java.text [v4]
Stuart Marks
smarks at openjdk.org
Thu Jul 6 23:40:56 UTC 2023
On Tue, 4 Jul 2023 22:03:58 GMT, John R Rose <jrose at openjdk.org> wrote:
>>> Hmm, I think that issue refers to code that have explicit non-Object parameter types (like `X::equals(Object)boolean` in the issue's sample). This method already have both arguments as `Object`, so I don't think there's any type-specific inlining opportunities.
>>
>> If that's true, then perhaps those (and some other) locations got that idea wrong:
>> * https://github.com/openjdk/jdk/blob/faf1b822d03b726413d77a2b247dfbbf4db7d57e/src/java.base/share/classes/java/util/Collections.java#L5712-L5719
>> * https://github.com/openjdk/jdk/blob/faf1b822d03b726413d77a2b247dfbbf4db7d57e/src/java.base/share/classes/java/util/AbstractMap.java#L577-L585
>>
>> Maybe @rose00 could clarify that?
>>
>> FWIW, I also note that `HashMap` does not use similar private static methods; it uses `Objects.equals(Object, Object)` and `Objects.hashCode` overloads that take parameters.
>
> I wrote a little case study on `Objects::equals` that talks about how it should optimize, when it does, why it doesn’t, and how (maybe) to fix that.
>
> https://cr.openjdk.org/~jrose/jvm/equals-profile.html
> https://cr.openjdk.org/~jrose/jvm/equals-profile.md
>
> This is also attached to the JBS bug.
>
> The work on [JDK-8026251](https://bugs.openjdk.org/browse/JDK-8026251) with the `TypeProfileLevel` switch bring us closer to correctly optimizing `Objects::equals` in more cases. Sadly, JDK refactoring by itself will not get all the way to where we want to go. The JVM’s profiling logic needs tweaking.
Thanks @rose00 for the writeup and @pavelrappo for asking pertinent followup questions.
For me the issue here is that there is a bunch of lore about avoiding `Objects::equals` and it's embodied in comments like this:
> NB: Do not replace with Object.equals until JDK-8015417 is resolved.
These comments are almost exactly ten years old, and we can't seem to find any evidence showing that a slowdown occurred if `Objects::equals` were used. The comments are a "dead hand" at this point. Is there a way to demonstrate whether there is or is not any difference when using `Objects::equals`?
As a side note, I observe that the `eq` method in Collections.java and AbstractMap.java does this:
return o1 == null ? o2 == null : o1.equals(o2);
whereas `Objects::equals` will test `o1 == o2` and skip the `equals()` call if the references are non-null and equals. This might confound the comparison a bit.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14752#discussion_r1255024667
More information about the core-libs-dev
mailing list