Re: RFR: JDK-8272192 Shortcut String equality checks by checking equality of the value array [v2]
On Mon, 6 Sep 2021 06:45:07 GMT, q2q-2q2
wrote:
>> Shortcut String equality checks by checking equality of the value array
>
> q2q-2q2 has updated the pull request incrementally with one additional commit
> since the last revision:
>
> JDK-8272192 Shortcut String equality checks by checking equality of the
> value array
David is right that many of these methods have VM intrinsics that'd have to be
fixed to see much benefit of this.
I can imagine some of these to be profitable in edge cases with string
de-duplication enabled. Otherwise the backing arrays will typically never be
shared and the identity test always be false (`""` is an exception to that rule
that interestingly is unlikely to see any win from short-cutting). While the
gain from a correct shortcut could be really large on a synthetic
micro-benchmark, I still suspect the added test might be more costly in
aggregate on the fabled "real application".
With compact strings it's easy to construct strings with different coders but
the same `byte[]`, and string de-duplication could be implemented so that you
end up sharing the same array between two semantically different strings (I
suspect this is the case, but I might be wrong). This means there's a possible
correctness issue in some places where you're shortcutting before checking
either length or coder.
All things considered I don't think this is worthwhile.
src/java.base/share/classes/java/lang/String.java line 1859:
> 1857: byte v1[] = value;
> 1858: byte v2[] = sb.getValue();
> 1859: if (v1 == v2) {
This one is a bit tricky since it's only correct since we've asserted that `len
== sb.length()`. It's unlikely to consistently give much of a boost, though,
since `sb.getValue()` is very likely to have some unused padding at the end.
-
PR: https://git.openjdk.java.net/jdk/pull/5370
Re: RFR: JDK-8272192 Shortcut String equality checks by checking equality of the value array [v2]
On Mon, 6 Sep 2021 06:45:07 GMT, q2q-2q2 wrote: >> Shortcut String equality checks by checking equality of the value array > > q2q-2q2 has updated the pull request incrementally with one additional commit > since the last revision: > > JDK-8272192 Shortcut String equality checks by checking equality of the > value array It isn't obvious to me that this is necessarily correct - do we only ever share the full array? IIRC at one time substrings also shared the same array but with different a starting index and length. If this is correct then it needs some supporting performance data, but as these methods are also VM intrinsics I doubt you will see much difference as only interpreted code will be affected by your change. But I'll leave it to the performance experts to comment - paging @cl4es :) Oh and as the JBS issue is filed against hotspot as the moment that needs to be cleaned up to, if this is just a Java fix. Cheers, David - PR: https://git.openjdk.java.net/jdk/pull/5370
Re: RFR: JDK-8272192 Shortcut String equality checks by checking equality of the value array [v2]
On Mon, 6 Sep 2021 06:41:38 GMT, q2q-2q2
wrote:
>> src/java.base/share/classes/java/lang/String.java line 1964:
>>
>>> 1962: public boolean equalsIgnoreCase(String anotherString) {
>>> 1963: if (anotherString != null) {
>>> 1964: return false;
>>
>> This one is definitely wrong.
>
> Thank you
So that begs the question as to what testing you did for this change?
-
PR: https://git.openjdk.java.net/jdk/pull/5370
Re: RFR: JDK-8272192 Shortcut String equality checks by checking equality of the value array [v2]
> Shortcut String equality checks by checking equality of the value array q2q-2q2 has updated the pull request incrementally with one additional commit since the last revision: JDK-8272192 Shortcut String equality checks by checking equality of the value array - Changes: - all: https://git.openjdk.java.net/jdk/pull/5370/files - new: https://git.openjdk.java.net/jdk/pull/5370/files/99d1e0f2..4fe9f85e Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5370&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5370&range=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/5370.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5370/head:pull/5370 PR: https://git.openjdk.java.net/jdk/pull/5370
Re: RFR: JDK-8272192 Shortcut String equality checks by checking equality of the value array
On Sat, 4 Sep 2021 11:59:58 GMT, q2q-2q2
wrote:
> Shortcut String equality checks by checking equality of the value array
src/java.base/share/classes/java/lang/String.java line 1964:
> 1962: public boolean equalsIgnoreCase(String anotherString) {
> 1963: if (anotherString != null) {
> 1964: return false;
This one is definitely wrong.
-
PR: https://git.openjdk.java.net/jdk/pull/5370
RFR: JDK-8272192 Shortcut String equality checks by checking equality of the value array
Shortcut String equality checks by checking equality of the value array - Commit messages: - JDK-8272192 Changes: https://git.openjdk.java.net/jdk/pull/5370/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5370&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8272192 Stats: 62 lines in 3 files changed: 60 ins; 1 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/5370.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5370/head:pull/5370 PR: https://git.openjdk.java.net/jdk/pull/5370
