[NEW BUG]: Avoid allocations in String.replace(CharSequence, CharSequence) in case no replacement happened

[Vitaly Davidovich]

Seems like a good idea.  You probably want to null check 'replacement'
before the bail out as the method is specified as throwing NPE in that case.
On Wed, Mar 1, 2017 at 4:37 PM Christoph Dreis <christoph.dreis at freenet.de>
wrote:

> Hey,
>
> I just noticed a small improvement for String.replace(CharSequence,
> CharSequence) in case the target sequence is not found.
>
> There is the possibility of an early-out in case there is nothing to
> replace. Yet there is a call to replacement.toString();
> While this is usually not a problem in case of String parameters, a
> StringBuilder - or other more "dynamic" CharSequence.toString()
> implementations - will likely produce unnecessary allocations here.
>
> I think the JDK could prevent that with the given patch below.
>
> I would be very happy if this is sponsored for JDK 10.
>
> Cheers,
> Christoph
>
> ===== PATCH ======
> diff --git a/src/java.base/share/classes/java/lang/String.java
> b/src/java.base/share/classes/java/lang/String.java
> --- a/src/java.base/share/classes/java/lang/String.java
> +++ b/src/java.base/share/classes/java/lang/String.java
> @@ -2177,11 +2177,11 @@
>       */
>      public String replace(CharSequence target, CharSequence replacement) {
>          String tgtStr = target.toString();
> -        String replStr = replacement.toString();
>          int j = indexOf(tgtStr);
>          if (j < 0) {
>              return this;
>          }
> +        String replStr = replacement.toString();
>          int tgtLen = tgtStr.length();
>          int tgtLen1 = Math.max(tgtLen, 1);
>          int thisLen = length();
>
> --
Sent from my phone