RFR: 8223147: JFR Backport (to jdk8u)

Sat Sep 14 18:02:04 UTC 2019

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
>