RFR: 8223147: JFR Backport (to jdk8u)

Wed Oct 16 19:48:56 UTC 2019

Hi all,

Apologies for the lateness on this, but we now have nightly builds
<https://ci.adoptopenjdk.net/job/build-scripts/job/jobs/job/jdk8u/job/jdk8u-linux-x64-hotspot-jfr/
>
(paused briefly for the CPU release).

What other platforms would folks like?

Cheers,
Martijn

On Sat, 14 Sep 2019 at 19:02, Martijn Verburg <martijnverburg at gmail.com>
wrote:

> Hi all,
>
> As an update we're finally getting JFR Linux builds out of Adopt - I'll
> update during OC1 with a download location for the nightly builds.
>
> If other platforms are desired please let me know, it's fairly easy to
> start expanding from here on in...
>
> Cheers,
> Martijn
>
>
> On Mon, 12 Aug 2019 at 08:56, Mario Torre <neugens at redhat.com> wrote:
>
>> On Mon, Aug 12, 2019 at 5:10 PM Andrey Petushkov <andrey at azul.com> wrote:
>> >
>> > Hi Mario, All,
>> >
>> > I've merged the initial patch with current state of incubator repo
>> (i.e. 8u222). Root repo and jdk patches applied cleanly.
>> > hotspot is not. The reasons are:
>> > - 8202353. It is already applied in upstream (see discussion in
>> parallel thread [1]). I did not apply respective part of the JFR patch
>>
>> That's fine, I understood you wanted to merge the jfr part as part of
>> the original request, if the code is not being backported we can just
>> backport 8202835 later.
>>
>> > - 8151539. jdk8u have no WeakProcessor (8189359 is not backported to
>> jdk8u) so in order to align the interfaces I've added overloaded
>> > version of Jfr::weak_oops_do. The diff is as follows:
>>
>> This https://bugs.openjdk.java.net/browse/JDK-8151539 was backported by
>> Alexey.
>>
>> Unfortunately at this moment the mercurial repo seems to be undergoing
>> a trantrum so it's a bit difficult to see what's in and what's no, but
>> I don't see the WeakProcessor code in my local copy, perhaps this was
>> not part of the backport as it's not in 8.
>>
>> The patches should be ok to push however.
>>
>> Cheers,
>> Mario
>>
>>
>>
>> > ====
>> > diff -U 3 ../jdk8u-jfr-incubator.1/hotspot/src/share/vm/jfr/jfr.cpp
>> hotspot/src/share/vm/jfr/jfr.cpp
>> > --- ../jdk8u-jfr-incubator.1/hotspot/src/share/vm/jfr/jfr.cpp
>>  2019-04-30 17:54:17.529600930 +0300
>> > +++ hotspot/src/share/vm/jfr/jfr.cpp    2019-08-12 17:38:46.842897757
>> +0300
>> > @@ -84,6 +84,11 @@
>> >    LeakProfiler::oops_do(is_alive, f);
>> >  }
>> >
>> > +void Jfr::weak_oops_do(OopClosure* f) {
>> > +  AlwaysTrueClosure always_true;
>> > +  LeakProfiler::oops_do(&always_true, f);
>> > +}
>> > +
>> >  bool Jfr::on_flight_recorder_option(const JavaVMOption** option, char*
>> delimiter) {
>> >    return JfrOptionSet::parse_flight_recorder_option(option, delimiter);
>> >  }
>> > diff -U 3 ../jdk8u-jfr-incubator.1/hotspot/src/share/vm/jfr/jfr.hpp
>> hotspot/src/share/vm/jfr/jfr.hpp
>> > --- ../jdk8u-jfr-incubator.1/hotspot/src/share/vm/jfr/jfr.hpp
>>  2019-04-30 17:54:17.529600930 +0300
>> > +++ hotspot/src/share/vm/jfr/jfr.hpp    2019-08-12 17:06:22.466382473
>> +0300
>> > @@ -52,6 +52,7 @@
>> >    static bool on_flight_recorder_option(const JavaVMOption** option,
>> char* delimiter);
>> >    static bool on_start_flight_recording_option(const JavaVMOption**
>> option, char* delimiter);
>> >    static void weak_oops_do(BoolObjectClosure* is_alive, OopClosure* f);
>> > +  static void weak_oops_do(OopClosure* f);
>> >    static Thread* sampler_thread();
>> >  };
>> > ====
>> >
>> > The invocations of Jfr::weak_oops_do have their first argument elided
>> so end up calling the newly added method,
>> > hence the functionality is not affected
>> >
>> > The rest of the patch applied cleanly. The same subset of jfr jtreg
>> passed on x86_64 as the one was passing with original patch. Ok to push?
>> >
>> > Thanks,
>> > Andrey
>> >
>> > [1]
>> http://mail.openjdk.java.net/pipermail/jdk8u-dev/2019-August/009986.html
>> >
>> > > On 3 Jun 2019, at 19:21, Mario Torre <neugens at redhat.com> wrote:
>> > >
>> > > On Mon, May 13, 2019 at 7:04 PM Andrey Petushkov <andrey at azul.com>
>> wrote:
>> > >>
>> > >> Hi Mario,
>> > >>
>> > >> Apparently the situation is not that bad, just a few things went
>> wrong on our sides.
>> > >> My fault I did not switch --enable-jfr to false. Fixed now
>> > >> And you seem did not make clean after changing configure options :)
>> With proper clean build the only wrong behavior I get from
>> > >> your list is the presence of jfc/template files in the distribution.
>> I've fixed that as well. I don't see any other JFR artefacts in the images
>> > >> directory when building with JFR disabled (there are few
>> intermediate files generated but these do not get into final image)
>> > >> Your test passes
>> > >> The webrevs are updated, please could you check again
>> > >
>> > > Hi Andrey,
>> > >
>> > > Thanks again for the patch. I did perform a bunch of tests and
>> > > although there is still work necessary, I think this part of the patch
>> > > is good to go, so please do commit it.
>> > >
>> > > As for the next steps, I'm going to see each of the patches that both
>> > > Azul and Alibaba submitted for review, I think we should create
>> > > subsequent umbrella bugs as we did with this first step so they are
>> > > easier to track.
>> > >
>> > > There is a number of fixes that is necessary once all of this has been
>> > > merged, for instance when running jtreg tests for the JFR directory I
>> > > noticed that many tests (about 30 at this first count) don't seem to
>> > > complete properly (one such example TestBiasedLockRevocationEvents).
>> > >
>> > > I didn't yet look at the specific of each test that fails but I think
>> > > we should be able to execute every test (or exclude the ones that fail
>> > > for obvious reason if necessary).
>> > >
>> > > Also, there's a number of differences between the jdk11 and onward
>> > > metadata.xml with what's in this backport patch. Denghui already
>> > > mentioned some unsupported events, but also the event definition in
>> > > some case is a mismatch. I'm going through each one of those and see
>> > > where this is part of the natural evolution of the events and what
>> > > should be instead changed to reflect the final definition.
>> > >
>> > > Nevertheless, I think it makes sense to start from here rather than
>> > > wait more to have one perfect patch, this will also give us a little
>> > > bit more exposure for doing performance test and the likes.
>> > >
>> > > So, bottom line, thanks for your patch, please go ahead and push to
>> > > the incubator repository!
>> > >
>> > > Cheers,
>> > > Mario
>> > >
>> > > --
>> > > Mario Torre
>> > > Associate Manager, Software Engineering
>> > > Red Hat GmbH <https://www.redhat.com>
>> > > 9704 A60C B4BE A8B8 0F30  9205 5D7E 4952 3F65 7898
>> >
>>
>>
>> --
>> Mario Torre
>> Associate Manager, Software Engineering
>> Red Hat GmbH <https://www.redhat.com>
>> 9704 A60C B4BE A8B8 0F30  9205 5D7E 4952 3F65 7898
>>
>