[JFR incubator] [RFR] JDK-8238590: Enable JFR by default during compilation in 8u

[Andrew John Hughes]

On 14/02/2020 12:40, Mario Torre wrote:
> On Fri, Feb 14, 2020 at 11:43 AM Ben Evans <bevans at newrelic.com> wrote:
>>
>> Hi Mario,
>>
>> Some of our teams have reported noticeable performance regressions on JDK8 + JFR even with JFR disabled at runtime.
>>
>> I am investigating further but don't have solid evidence that I can share yet - but I would urge everyone who wants this backport to happen (at NR we very much do) to do as much performance regression testing as possible. It would not be a good look if adding this feature caused problems for other users.
>>
> 
> Thanks Ben,
> 
> I am about to prepare a new webrew for general testing based on all
> the latest fix we introduced in the last couple of weeks, I appreciate
> this kind of feedback on the new bundle, especially if it can be
> substantiated by some numbers.
> 
> Cheers,
> Mario
> 
> 

If the plan is now to aim JFR for the July update, I'd suggest waiting
until we fork for rampdown before preparing a new merge. Such merges
date very quickly with new changes going into 8u, so it should be done
as close to potential integration as possible.

Note that it should be a *merge* so we keep the incubator history, not
some mega-patch.

I was going to comment on the original merge thread, but may as well say
it here instead. Such integrations shouldn't be a review of the
technical details of the changes. That should have been done on each
individual change before integration into the incubator, and the reason
to retain that history is to keep a clear mapping to that review process.

I would expect considerations for integration to be on a higher level:

1. Do we want to integrate this change in the current cycle?

2. If integrated, what are the potential consequences?

3. Is the feature enabled by default?

4. What fallback options do we have if issues are discovered?

My personal feeling is that we should get JFR into 8u early in the 8u262
cycle, which I expect to begin in early March. Only a limited audience
will have been testing the incubator (I know fixing the 7 bootstrap was
my first experiment with it), so we need to maximise the testing
timeframe if we go ahead for 8u262.

As to enabling by default, I would suggest that we delay this until the
October cycle, so we have one cycle with the code present, but manually
enabled, and then spend the 8u272 cycle explicitly testing the effects
of default enablement. I think having it on by default is the right
path, but there's no need to rush it.

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
https://keybase.io/gnu_andrew