RFR: 8245455: Remove alternative StringConcatFactory strategies

[Aleksey Shipilev]

On 5/22/20 2:16 PM, Claes Redestad wrote:
> What I'm asking for is whether there exist any operational issues that
> means users have to use any of the fallback strategies in the wild on
> JDK 11/14 today. That would be a good argument to keep maintaining the
> code, but I've not heard of such a case.

Well, I myself thought there are "no operational issues" with string concat too for years after it
was in. Until at least two major bugs (one is this filter-order issue, and other is "+=" issue) were
discovered. This makes me extremely averse of sticking with a single strategy. And I think you
should be averse too!

We know that good JVMs have runtime killswitch options to be used when something goes wrong. The
killswitch might be removed after we are definitely sure most users migrated to the common path, and
after that happens, no issues were reported for a while. As it stands right now, the migration from
8 is far, far, far from over, and so I would not discount the possibility new issues come up during
that migration.

Having no fallback means issue in MH_* string concat strategy would be a showstopper. Having a very
simple fallback strategy today is a responsible thing to do, even if it makes maintenance harder. I
understand the maintainability argument, and I think that removing most strategies strikes the
balance between maintainability and the width of the reliability safety net.

Let's just not remove all-but-one-we-hope-is-right.

> For testing and verifying the correctness of dynamic concat callsites we
> have the javac fallback to emit SB chains (-XDstringConcat=inline) -
> maintaining another fallback strategy is duplication of effort and a
> source of potential bugs in its own right.

-XDstringConcat is the compilation time killswitch, it is not the same. I don't think we would ask
users to recompile the JARs they got from Maven Central to deploy the workaround. Runtime option
that enables a fallback strategy is what is needed for field deployment when things go very wrong.

-- 
Thanks,
-Aleksey