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

[Andrew John Hughes]

On 14/02/2020 17:39, Mario Torre wrote:
> Il giorno ven 14 feb 2020 alle ore 18:16 Andrew John Hughes
> <gnu.andrew at redhat.com> ha scritto:
> 
>> 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.
> 
> Yes, I agree and this was my original idea, but during the Committers
> workshop was specifically asked to enable it by default right away.
> Nonetheless, I don't mind either way :)
> 
> I can skip this change if so agreed.
> 

I'm not suggesting to skip it. I'm suggesting it should happen after the
release of 8u262, so there is a release with the code integrated, but
disabled by default.

I see no rush into turning it on for everyone by default.

If someone wants to make the case for that, they are welcome to.

> Btw, I'm not sure I understand what's the difference between a merge
> and a plain hg import of all those patches exported from
> jdk-incubator, history and everything are still preserved since they
> are part of the bug id anyway and are in this mailing list, no?

A lot of unnecessary work?

I really don't understand why you'd not simply merge from the repository
where this has been baking, rather than basically starting the whole
process again. It seems to defeat the point of having the incubator
repository altogether, and frankly, seems bizarre to me.

> 
> Also, we never update the jfr-incubator exactly in order to apply the
> patches on top of the target version and rework if/what is necessary,
> I think this is a better strategy but I understand maintainers may
> have different preferences here; finally, I agree those patches should
> have been (and indeed were!!) reviewed during the last year, and they
> are also used in production by Azul and Alibaba, but still given the
> scope of the change an extra pair of eyes is welcomed, so while a full
> review isn't necessary, I hope to have feedback (by you, Andrew
> Dinn/Haley and whoever else has experience and knowledge of hotspot
> code), especially on the subtleties of some of this code, and even
> more so on the performance aspect, which is the one feedback we lack
> the most since, as you noted, not many have been playing with the
> incubator thus far.
> 

i think, now the incubator is relatively stable and preparing for
integration, you should look into doing regular merges with 8u-dev. That
would enable you to use this time before we fork for 8u252 to test with
the latest changes and will minimise the eventual merge with 8u-dev on
integration. In my experience, regular, small merges are easier to work
with, so I'd strongly suggest each build drop from here on in, starting
with b03, which I'm currently testing.

Note that, last night, I removed the jdk8u-jdk-request tags from the
bugs and instead linked them to a metabug [0]. When you are ready to
integrate, please flag that bug with jdk8u-fix-request rather than
fifty-odd individual issues, which made our approval queue unusable for
other issues.

> Cheers,
> Mario
> 

[0] https://bugs.openjdk.java.net/browse/JDK-8239140

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