[PATCH] unnecessary null check inside of java.lang.Boolean.valueOf(String)

[Сергей Цыпанов]

Hi,

currently (JDK 9.0.4) j.l.Boolean.valueOf(String) contains this code:

public static boolean parseBoolean(String s) {
  return ((s != null) && s.equalsIgnoreCase("true"));
}

Null check here seems to be redundant, as String.equalsIgnoreCase does it itself:

public boolean equalsIgnoreCase(String anotherString) {
  return this == anotherString ? true
  : anotherString != null
    && anotherString.length() == length()
    && regionMatches(true, 0, anotherString, 0, length());
}

So we can rid this check making

public static boolean parseBoolean(String s) {
  return "true".equalsIgnoreCase(s));
}

which is more simple and faster. Consider benchmark:

@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
public class EqualsIgnoreCaseBenchmark {

    @Benchmark
    public boolean defaultMethod(Data data) {
        return data.str != null && data.str.equalsIgnoreCase("true");
    }

    @Benchmark
    public boolean betterMethod(Data data) {
        return "true".equalsIgnoreCase(data.str);
    }

    @State(Scope.Thread)
    public static class Data {
        @Param({"true", "false", "null"})
        String str;

        @Setup
        public void setup() {
            str = "null".equals(str) ? null : str;
        }
    }
}

This benchmark on my machine (openjdk version "1.8.0_162", Intel(R) Core(TM) i5-4690) gives the following results:

Benchmark                                (str)  Mode  Cnt  Score   Error  Units
EqualsIgnoreCaseBenchmark.betterMethod    true  avgt   50  5.095 ± 0.405  ns/op
EqualsIgnoreCaseBenchmark.defaultMethod   true  avgt   50  9.113 ± 0.212  ns/op

EqualsIgnoreCaseBenchmark.betterMethod   false  avgt   50  2.973 ± 0.076  ns/op
EqualsIgnoreCaseBenchmark.defaultMethod  false  avgt   50  2.842 ± 0.017  ns/op

EqualsIgnoreCaseBenchmark.betterMethod    null  avgt   50  2.395 ± 0.020  ns/op
EqualsIgnoreCaseBenchmark.defaultMethod   null  avgt   50  2.232 ± 0.020  ns/op

Also we don't need to do exlicit null check for argument of Boolean.parseBoolean(String) e.g. in jdk.internal.module.ModuleBootstrap, line 302.

Here's the patch:

diff --git a/src/java.base/share/classes/java/lang/Boolean.java b/src/java.base/share/classes/java/lang/Boolean.java
--- a/src/java.base/share/classes/java/lang/Boolean.java
+++ b/src/java.base/share/classes/java/lang/Boolean.java
@@ -129,7 +129,7 @@
      * @since 1.5
      */
     public static boolean parseBoolean(String s) {
-        return ((s != null) && s.equalsIgnoreCase("true"));
+        return "true".equalsIgnoreCase(s);
     }
 
     /**
@@ -238,7 +238,7 @@
      */
     public boolean equals(Object obj) {
         if (obj instanceof Boolean) {
-            return value == ((Boolean)obj).booleanValue();
+            return value == (Boolean) obj;
         }
         return false;
     }
diff --git a/src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java b/src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java
--- a/src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java
+++ b/src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java
@@ -299,7 +299,7 @@
 
         PrintStream traceOutput = null;
         propValue = getAndRemoveProperty("jdk.module.showModuleResolution");
-        if (propValue != null && Boolean.parseBoolean(propValue))
+        if (Boolean.parseBoolean(propValue))
             traceOutput = System.out;
 
         // run the resolver to create the configuration


Regards,
Sergey Tsypanov