[11u]: RFR: 8215281: Use String.isEmpty() when applicable in java.base

[Thomas Stüfe]

Hi Christoph,

I looked at the changes. Mostly good - true to the original change.

At various places
(e.g. src/java.base/share/classes/java/lang/ClassLoader.java) one could
remove the extra brackets, but since we want to keep the diff to head at a
minimum where possible I would not do this.

However, your patch (and the original one) contained the following part I
do not understand:

http://cr.openjdk.java.net/~clanger/webrevs/8215281.11u/src/java.base/windows/classes/sun/nio/ch/FileDispatcherImpl.java.udiff.html

     static boolean isFastFileTransferRequested() {
         String fileTransferProp = GetPropertyAction
-                .privilegedGetProperty("jdk.nio.enableFastFileTransfer");
-        boolean enable;
-        if ("".equals(fileTransferProp)) {
-            enable = true;
-        } else {
-            enable = Boolean.parseBoolean(fileTransferProp);
-        }
-        return enable;
+                .privilegedGetProperty("jdk.nio.enableFastFileTransfer",
"false");
+        return fileTransferProp.isEmpty() ? true :
Boolean.parseBoolean(fileTransferProp);
     }

The old version called GetPropertyAction::privilegedGetProperty() without
default value. The new versions passes "false" as default value. I fail to
see how this could ever return an empty string? Also, would that not change
default behavior?

Note that this is part of the original change.

Cheers, Thomas





On Tue, Jun 18, 2019 at 10:59 AM Langer, Christoph <christoph.langer at sap.com>
wrote:

> Hi,
>
> please review the backport of the String.isEmpty() cleanups in java.base.
>
> The main reason to bring this down to JDK11u is that it'll ease
> backporting of later changes which otherwise run into conflicts and won't
> resolve. Furthermore, the original change is a nice cleanup and improves
> performance.
>
> Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8215281.11u/
> Bug: https://bugs.openjdk.java.net/browse/JDK-8215281
> Change in jdk/jdk: http://hg.openjdk.java.net/jdk/jdk/rev/8bf9268df0e2
>
> The original patch merely applies (with a little fuzz in some places).
> There were 4 rejects:
>
> src/java.base/share/classes/java/lang/VersionProps.java.template
> --- VersionProps.java.template
> +++ VersionProps.java.template
> @@ -95,7 +95,7 @@
>          props.put("java.version.date", java_version_date);
>          props.put("java.runtime.version", java_runtime_version);
>          props.put("java.runtime.name", java_runtime_name);
> -        if (VENDOR_VERSION_STRING.length() > 0)
> +        if (!VENDOR_VERSION_STRING.isEmpty())
>              props.put("java.vendor.version", VENDOR_VERSION_STRING);
>          props.put("java.class.version", CLASSFILE_MAJOR_MINOR);
>
> -> manually resolved that to the right location
>
> src/java.base/share/classes/java/text/CompactNumberFormat.java
>
> -> file does not exist in 11u, so dropping
>
>
> src/java.base/share/classes/jdk/internal/org/objectweb/asm/util/CheckMethodAdapter.java
> --- CheckMethodAdapter.java
> +++ CheckMethodAdapter.java
> @@ -1314,7 +1314,7 @@
>        * @param message the message to use in case of error.
>        */
>      static void checkMethodIdentifier(final int version, final String
> name, final String message) {
> -        if (name == null || name.length() == 0) {
> +        if (name == null || name.isEmpty()) {
>              throw new IllegalArgumentException(INVALID + message +
> MUST_NOT_BE_NULL_OR_EMPTY);
>          }
>          if ((version & 0xFFFF) >= Opcodes.V1_5) {
> @@ -1347,7 +1347,7 @@
>        * @param message the message to use in case of error.
>        */
>      static void checkInternalName(final int version, final String name,
> final String message) {
> -        if (name == null || name.length() == 0) {
> +        if (name == null || name.isEmpty()) {
>              throw new IllegalArgumentException(INVALID + message +
> MUST_NOT_BE_NULL_OR_EMPTY);
>          }
>          if (name.charAt(0) == '[') {
> @@ -1457,7 +1457,7 @@
>        * @param descriptor the string to be checked.
>        */
>      static void checkMethodDescriptor(final int version, final String
> descriptor) {
> -        if (descriptor == null || descriptor.length() == 0) {
> +        if (descriptor == null || descriptor.isEmpty()) {
>              throw new IllegalArgumentException("Invalid method descriptor
> (must not be null or empty)");
>          }
>          if (descriptor.charAt(0) != '(' || descriptor.length() < 3) {
>
> -> file looks different in 11u due to a missing change. So, modified the
> right places in the 11u version
>
>
> src/java.base/share/classes/jdk/internal/org/objectweb/asm/util/CheckSignatureAdapter.java
> --- CheckSignatureAdapter.java
> +++ CheckSignatureAdapter.java
> @@ -365,7 +365,7 @@
>      }
>      private void checkClassName(final String name, final String message) {
> -        if (name == null || name.length() == 0) {
> +        if (name == null || name.isEmpty()) {
>              throw new IllegalArgumentException(INVALID + message + "
> (must not be null or empty)");
>          }
>          for (int i = 0; i < name.length(); ++i) {
> @@ -377,7 +377,7 @@
>      }
>      private void checkIdentifier(final String name, final String message)
> {
> -        if (name == null || name.length() == 0) {
> +        if (name == null || name.isEmpty()) {
>              throw new IllegalArgumentException(INVALID + message + "
> (must not be null or empty)");
>          }
>          for (int i = 0; i < name.length(); ++i) {
>
> -> file looks different in 11u due to a missing change. So, modified the
> right places in the 11u version
>
> Thanks
> Christoph
>
>