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
