[11u] RFR(S): 8241234: Unify monitor enter/exit runtime entries.

[Andrew Haley]

Hi,

On 28/08/2020 14:11, Lindenmaier, Goetz wrote:
>
>>> I'm not really happy with 11 staying behind 11-oracle in the JVMCI issue.
>> What JVMCI issue is this? Please explain. All that I see is a faster
>> "slow" locking path for monitors.
>
> This was meant as a more general comment. I wanted to address that
> we don't integrate many of the JVMCI changes so the OpenJDK 11 is
> probably not usable with graal.  The comment was not tailored to
> this specific change.  Unfortunately our team has not the capacity
> to look at JVMCI/graal.

Fair enough.

Now, let's think about the wider point.

Any change is bad because our users want, above all else,
stability. So first we should avoid change.

In order to justify any change, I want backport patches to have a real
justification. That is to say, they must have a real effect on a Java
user's experience.  Fixing visible bugs obviously qualifies, as does a
significant performance bump, as does meeting a new crypto
specification, etc, etc.

The other good reason is improved stability, which includes better
testing.

A real justification doesn't exclude "cleanups", as long as there is
some other benefit, such as making making a proposed backport
cleaner. But it has to be a backport that we are actually doing, not
some unknown backport that might happen some day.

It may well be that the 8241234 fix has a definite performance
advantage, in which case it might be a reasonable thing to do.
The provided justifications were:

- Oracle has done so. There may be more backports in this area and I'd
  expect less effort if we have the same code in the open version.
- Performance is supposed to be better.
- New code is much cleaner.

But even though the new code is much cleaner, it's a significant
change in a very delicate area. Bugs in this are can take a long time
to reveal themselves, usually under heavy load in a production
situation.

I am not saying no to this patch. I am asking "Are you sure that this
change is worth making the change?" Given that I doubt anyone will
ever notice this change unless it breaks something important, I have
my doubts.

So, anyone: is there any chance that this patch will break something?
Is this change worth the churn?

-- 
Andrew Haley  (he/him)
Java Platform Lead Engineer
Red Hat UK Ltd. <https://www.redhat.com>
https://keybase.io/andrewhaley
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671