RFR (S) 8151223: String concatenation fails with implicit toString() on package-private class

[Aleksey Shipilev]

On 03/08/2016 02:25 PM, Maurizio Cimadamore wrote:
> Here's an extreme case where sharpening can end up in some weird case in
> a binary compatible scenario; quoting chapter 13:
> 
> "Changing the direct superclass or the set of direct superinterfaces of
> a class type will not break compatibility with pre-existing binaries,
> provided that the total set of superclasses or superinterfaces,
> respectively, of the class type loses no members."
> 
> So, let's start with:
> 
> public class A { }
> 
> class Inaccessible extends A { }
> 
> Here, the sharpening method would lift 'Inaccessible' to 'A'.
> 
> Now, let's change that as follows:
> 
> public class A { }
> public class B extends A { }
> 
> class Inaccessible extends B { }
> 
> Now, the sharpening method will lift 'Inaccessible' to 'B'.
> 
> Is this an issue? While I don't think it's a big issue, it also means
> that the runtime cannot totally rely on the static information put there
> by javac, as this information might as well be stale (as in the above
> case, where a classfile says 'A' where in fact the sharpest type is
> 'B'). Of course this can happen even w/o accessibility woes, so we
> should probably not worry about this?

To be fair, lifting from the inaccessible class to some other class
already loses "sharpness". There are shades of gray in what's the exact
class chosen between java.lang.Object and the leaf inaccessible class.
IOW, we are talking about heuristics here, and a heuristic still blindly
choosing A after B had appeared is well-behaved.

In yet another words, the type information in BSM call is already a
guess -- still a useful one. Current patch basically says that *some*
type information is still better than the absolute absence of it.  The
ISC code would still happily work with A. Recompilation would resharpen
to B.

Now, the design goal for ISC was to come up with a stable bytecode that
would not force users to recompile their Java programs when we change
the String concat strategy. Whether that should be extended to
binary-compatible changes in concat inputs, like in the example above,
is debatable. I'd say no; you face the entire realm of new performance
issues when you extend the class hierarchy in a binary compatible manner
(think new cases in type profiling), and this is yet another little thing.

Of course, since we are facing this issue with a inaccessible classes
only (which are, arguably, very rare in string concats), we might as
well strip them to Object. But, I am reluctant of giving up type
information without being forced to do so.

Aside: spelling out the actual types before calling toString on them in
the concat stub is actually better for optimizers, because it bounds the
set of possible classes there at the .toString() invocation site.
Ultimately, for public leaf classes, you may skip type profiling and/or
inline caching altogether (CHA does it already). The "smart compiler
card" has less power here, because ISC is basically an attempt to
downplay the importance of complicated optimizing compiler, in favor of
smarter handling in JDK.

Thanks,
-Aleksey

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20160308/affd7a9d/signature-0001.asc>