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

[Ulf Zibis]

Am 02.03.2017 um 14:55 schrieb Vitaly Davidovich:
> Implicit null checks ... generally don't cost anything.
Any additional byte of code in CPU cache prevents other code from staying there.

>
>     I can't imagine, if any programmer would rely on such contract, instead of doing the check
>     explicitly if of significance.
>
> I think it's to have consistent behavior, irrespective of control flow.  It can detect bugs early on.
Yes, serving bug detection almost always has some cost and should be pondered, but the issue of this 
patch is to increase performance. Here I suspect the real value of the bug detection help.

>     I think, the original reasonable of the contract is to rule out any other exception type or
>     undefined behaviour in case of relevance of the argument. So I would vote for stating the contract
>     more precisely in that way.
>
>     Also, if I remember correct, if a variable, here replStr, is only referenced later, the
>     compiler can
>     move the execution of here replacement.toString() to a later position, so it is not clear to
>     me, if
>     the original implementation ever fulfilled the contract.
>
> Compiler can only do that if it proves a bunch of things, such as no observable side effects as a 
> result of sinking it down.  If CharSequence.toString isn't devirtualized there, eg, it definitely 
> cannot do it.  I wouldn't bet on it.
For the original code in case of inlining, I guess, the optimizer would place the null check before 
the if block and the allocation of the replacement String after. For the new code, the optimizer can 
detect the duplicate null check inside and outside the if block, so can move it before the if block. 
So both java codes could result in the same machine code. IMHO the effective optimization should be 
proved by HS disassembler.

There is also the possibility, that an allocation of an intermediate String object is never done, 
because the anyway involved StringBuilder perhaps can insert the replacement sequence without before 
instantiating a String object.

-Ulf