[8u] RFR 8233197: Invert JvmtiExport::post_vm_initialized() and Jfr:on_vm_start() start-up order for correct option parsing

[Jaroslav Bachorík]

Hi Andrew,

On Mon, Jun 8, 2020 at 5:08 PM Andrew Hughes <gnu.andrew at redhat.com> wrote:
>
> On 08/06/2020 11:34, Jaroslav Bachorík wrote:
>
> snip...
>
> >>
> >> My initial thought is that, if a patch takes a long time in review,
> >> that's probably a sign that it required a lot of debate and alteration,
> >> and so shouldn't be going into a release late, rather than the opposite.
> >
> > I do not think this is a good example of a contentious issue. The RFR
> > spent most of its time being idle waiting for review - the only issues
> > were related to formatting and non-existent test (which does not exist
> > upstream either). So the point about debate and alteration is rather
> > moot here.
> >
>
> Ok. I was speaking generally with regard to your comment "this patch
> missed the cutoff date for 8u262 mostly due to review". In many cases,
> that alone would be a reason not to include, rather than to include.
>
> I haven't followed the discussion that closely, but what you say about
> this particular patch seems acceptable.
>
> >>
> >> However, I'm of course open to hearing why this is critical to the 8u262
> >> release and can't wait until 8u272.
> >
> > Well, not having JVM segfault on a valid use of JFR would be nice. But
> > it's your call as the maintainer.
> >
>
> Well, I need information to make such a decision :-)
>
> So it would help if you could answer the following:
>
> 1. How prolific is such usage of JFR?

I guess not much since till recently each attempt would have resulted
in segfault.
But in general I would say that any APM/Profiler based on JFR would be
happy not to run a special delay code to prevent segfaults.

>
> 2. What are the risks of including this patch?

In general, changing the initialization order for java.lang.Class
could cause problems for a code that would operate with assumption of
the java.lang.Class not being initialized very early on in the JVM
startup. This sounds pretty unlikely, though - eg tier1, tier2 and jfr
tests are still passing after adding this change


>
> 3. Do we now have a final version of this patch, with test?

Yes. The webrev.04 version is the final one.


>
> 4. Was a bug filed for the additional test?

https://bugs.openjdk.java.net/browse/JDK-8246703


>
> I've no issue with this going in before the test makes it into later JDK
> versions, but we should be tracking that this happens.

Sure, will do.

Cheers,

-JB-

>
> Thanks,
> --
> Andrew :)
>
> Senior Free Java Software Engineer
> Red Hat, Inc. (http://www.redhat.com)
>
> PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net)
> Fingerprint = 5132 579D D154 0ED2 3E04  C5A0 CFDA 0F9B 3596 4222
>