RFR 8214971 : Replace use of string.equals("") with isEmpty()

[Claes Redestad]

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 
>> just replace it all while we're at it.
>
> Interesting. The compact strings stuff saves a lot of space, but it 
> means that more setup work needs to be done before an actual equality 
> comparison can be done. Thus, equals("") has gotten quite a bit more 
> expensive relative to isEmpty() compared with the situation prior to 
> compact strings.
>
> Still, it seems like a method call could be saved here:
>
>         if (anObject instanceof String) {
>             String aString = (String)anObject;
>             if (coder() == aString.coder()) {
>                 return isLatin1() ? StringLatin1.equals(value, 
> aString.value)
>                                   : StringUTF16.equals(value, 
> aString.value);
>             }
>         }
>
> by saving the result of coder() in a local variable and then comparing 
> it directly to LATIN1. Is it worth it?

the reason this might be a bad idea is that guarding the coder field 
reads in
methods that first test whether COMPACT_STRINGS is enabled enables the JIT
to better optimize away untaken code paths. This ensures optimal as can be
performance for the different run modes. See javadoc for COMPACT_STRINGS
in String.java[1] for some discussion on this.

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 optimizable, so less taxing 
during
startup with no peak performance drawback.

>
> (Note, this isn't relevant to the current review.)

But interesting nonetheless :-)

/Claes

[1]

/** * If String compaction is disabled, the bytes in {@code value} are * 
always encoded in UTF16. * * For methods with several possible 
implementation paths, when String * compaction is disabled, only one 
code path is taken. * * The instance field value is generally opaque to 
optimizing JIT * compilers. Therefore, in performance-sensitive place, 
an explicit * check of the static boolean {@code COMPACT_STRINGS} is 
done first * before checking the {@code coder} field since the static 
boolean * {@code COMPACT_STRINGS} would be constant folded away by an * 
optimizing JIT compiler. The idioms for these cases are as follows. * * 
For code such as: * * if (coder == LATIN1) { ... } * * can be written 
more optimally as * * if (coder() == LATIN1) { ... } * * or: * * if 
(COMPACT_STRINGS && coder == LATIN1) { ... } * * An optimizing JIT 
compiler can fold the above conditional as: * * COMPACT_STRINGS == true 
=> if (coder == LATIN1) { ... } * COMPACT_STRINGS == false => if (false) 
{ ... } * * @implNote * The actual value for this field is injected by 
JVM. The static * initialization block is used to set the value here to 
communicate * that this static final field is not statically foldable, 
and to * avoid any possible circular dependency during vm 
initialization. */