Hi Claes,
the difference is the "array encoding" (ae) argument passed to the intrinsic.
See
MacroAssembler::string_compare(... int ae). If we compare an UTF16 String, we
know that the number
of bytes is always even (two byte / char encoding), whereas a Latin1 string has
a byte encoding. So
Hi Claes,
Early in the design there was a question about whether the coder was in
the array
or the String. That probably translated into following the general pattern
that the UTF16 coding was nearly identical and parallel to Latin1
with minimal differences for the size of the data.
$.02,
This is an interesting point: can anyone explain why there are two
distinct methods for LATIN1 and UTF16 equality, with corresponding
intrinsics? Aleksey? Tobias?
Testing with Arrays.equals then performance profile is
quite different due vectorization (often better, sometimes worse),
but this
On 12/7/18 4:51 AM, Claes Redestad wrote:
One possible improvement would to wrap coder() == aString.coder() in a
method isSameCoder(String):
private boolean isSameCoder(String other) {
return COMPACT_STRINGS ? coder == other.coder : true;
}
.. one less method call, but still perfectly
Hi Hannes,
I'd take that up as a separate issue. [1]
A nice thing about the current patch is that the change is uniform and
easier to review.
It would also be good to look at a utility method that efficiently does
the combined check.
Thanks, Roger
[1] 8215014
Hi,
On 2018-12-07 03:56, Stuart Marks wrote:
On 12/6/18 2:42 PM, Claes Redestad wrote:
I filed this bug after seeing it in startup profiles, where isEmpty()
avoids an instanceof and four(!) method calls, so getting rid of a
few of these has a measurable impact on startup. It seemed prudent to
Hi Hannes,
On 07/12/2018 09:24, Hannes Wallnöfer wrote:
Hi Roger,
There are quite a few occurrences of the reverse calling pattern "".equals(str)
in core libs.
This is a bit more tricky because it allows str to be null, but when used
following a null/not-null check it could be replaced with
Hi Roger,
There are quite a few occurrences of the reverse calling pattern "".equals(str)
in core libs.
This is a bit more tricky because it allows str to be null, but when used
following a null/not-null check it could be replaced with isEmpty() as well.
I wonder if these should be included
Or simply;
if (anObject instanceof String) {
String aString = (String)anObject;
if (coder() == aString.coder())
return Arrays.equals(value, aString.value);
}
}
Sent from my iPhone
> On Dec 6, 2018, at 10:56 PM, Stuart Marks wrote:
>
>>
Hi,
I'm pretty sure, it is assumed that the JVM compiler will optimize this code
and produce an optimal result. Lots of perf work went into the string
size optimizations.
Also possible is to compare the value.length values before dispatching
to Latin1 vs UTF16.
For unequal length strings,
On 12/6/18 2:42 PM, Claes Redestad wrote:
I filed this bug after seeing it in startup profiles, where isEmpty() avoids an
instanceof and four(!) method calls, so getting rid of a few of these has a
measurable impact on startup. It seemed prudent to just replace it all while
we're at it.
+1
> On Dec 6, 2018, at 6:03 PM, Roger Riggs wrote:
>
> I missed reverting other Apache source files that should be unchanged.
>
> Webrev:
> http://cr.openjdk.java.net/~rriggs/webrev-string-isempty-8214971-3/
>
> Thanks, Roger
>
>
> On 12/6/18 5:37 PM, Roger Riggs wrote:
>> Hi Claes,
>>
>>
On 2018-12-07 00:03, Roger Riggs wrote:
Webrev:
http://cr.openjdk.java.net/~rriggs/webrev-string-isempty-8214971-3/
Looks good!
/Claes
I missed reverting other Apache source files that should be unchanged.
Webrev:
http://cr.openjdk.java.net/~rriggs/webrev-string-isempty-8214971-3/
Thanks, Roger
On 12/6/18 5:37 PM, Roger Riggs wrote:
Hi Claes,
Thanks for the reminder about xalan sources.
Updated to revert changes to xalan.
Hi Claes,
Thanks for the reminder about xalan sources.
Updated to revert changes to xalan.
Webrev:
http://cr.openjdk.java.net/~rriggs/webrev-string-isempty-8214971-2/
Thanks, Roger
On 12/6/18 4:48 PM, Claes Redestad wrote:
Should changes to various com.sun.org.apache.xalan classes in
I filed this bug after seeing it in startup profiles, where isEmpty()
avoids an instanceof and four(!) method calls, so getting rid of a few
of these has a measurable impact on startup. It seemed prudent to just
replace it all while we're at it.
C2 should be able to optimize equals("") to the
On 12/6/2018 1:08 PM, Pavel Rappo wrote:
Roger, changes look good. On a side note, it's impressive how many cases we have
when we check for string not being null and empty. We should have had something
for that *internally* in the JDK.
Or perhaps that should be a utility method on String or
Hi Jim,
Yes, it is likely that the compiler gets to the same optimization.
But we save a little bit too by not making the compiler work so hard.
And it helps during startup before the compiler kicks in.
IntelliJ did the hard work and a full CI build/test is in progress.
And isEmpty reads
Should changes to various com.sun.org.apache.xalan classes in java.xml
be deferred to upstream?
Otherwise looks good to me!
/Claes
On 2018-12-06 21:04, Roger Riggs wroe te:
Please review changing string.equals("") to string.isEmpty().
isEmpty is preferred, performs better and is easier to
Roger, changes look good. On a side note, it's impressive how many cases we have
when we check for string not being null and empty. We should have had something
for that *internally* in the JDK. And it's a pity that `String.trim().isEmpty()`
not equivalent to `String.isBlank`. Seems to be a
Hi Roger,
The changes look good.
best regards,
-- daniel
On 06/12/18 20:04, Roger Riggs wrote:
Please review changing string.equals("") to string.isEmpty().
isEmpty is preferred, performs better and is easier to optimize.
The change is a literal substitution in all files and only in these
HI Roger,
Changes seem good
> On Dec 6, 2018, at 3:04 PM, Roger Riggs wrote:
>
> Please review changing string.equals("") to string.isEmpty().
> isEmpty is preferred, performs better and is easier to optimize.
>
> The change is a literal substitution in all files and only in these modules:
>
> On 6 Dec 2018, at 20:27, Jim Laskey wrote:
>
> I assume you automated this change, might be worthwhile to automate a
> verification (compare the -+ pairs.) Eyeballing the patch it seems complete,
> one comment changed as well.
>
> Question: Is it probably that vm optimizes .equals(“”) to
I assume you automated this change, might be worthwhile to automate a
verification (compare the -+ pairs.) Eyeballing the patch it seems complete,
one comment changed as well.
Question: Is it probably that vm optimizes .equals(“”) to .isEmpty() and there
is no net win, ie., more of an esthetic
Please review changing string.equals("") to string.isEmpty().
isEmpty is preferred, performs better and is easier to optimize.
The change is a literal substitution in all files and only in these modules:
java.base
java.logging
java.management
java.management.rmi
java.naming
25 matches
Mail list logo
