RFR: 8245455: Remove alternative StringConcatFactory strategies

[Claes Redestad]

On 2020-05-22 14:40, Aleksey Shipilev wrote:
> 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!

There are no guarantees any particular fallback is not going to break in
similar ways or - more likely - bit rot and break sooner. Seeing how all
but the default strategy sees rather superficial testing in practice, I
think you are exaggerating the value they provide as a safety net
against future unknowns.

Fixing some bootstrap robustness issues of late has me convinced that
maintaining and verifying the fallback strategies is not worth our
while. See e.g. https://bugs.openjdk.java.net/browse/JDK-8218173 -
fixing this for the fallback strategies might be straightforward (or
not), but to then verify it's actually fixed would require we add runs
with $chosen_fallback_strategy to our entire testing for weeks.

So no, I'm not particularly worried about reducing this down to a single
strategy here and now. We should of course keep investing in SCF to
ensure it's as robust, performant and well-tested as can be. And of
course promptly fix any bugs that we discover.

> 
> 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.
> 

I only suggested recompiling with -XDstringConcat=inline as a way to
verify and test behavior.

/Claes