On Thu, Mar 2, 2017 at 11:42 AM, Ulf Zibis <ulf.zi...@cosoco.de> wrote:
> > 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. When the optimization applies, there shouldn't be *any* instructions for the null check on the path - it's done via signal handling. > > > >> 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. Performance by way of removing an allocation. There's already code in that method that's much more expensive than a test against a register. So while I suggested to move the explicit null check into the if block, I don't think it'll really matter. But removing an allocation is always a good thing. > > > 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. > This is Sufficiently Smart Compiler territory. It's much better to hand-code this properly, particularly since nothing is lost in readability or maintainability in this case. > > 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. I know C2 can remove some intermediate string allocations, but I would be very impressed if it handled this particular method in the way you describe. > > > -Ulf > >
