RFR: 8223147: JFR Backport (to jdk8u)

Andrey Petushkov andrey at azul.com
Mon Aug 12 15:10:16 UTC 2019


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
- 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:
====
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



More information about the jdk8u-dev mailing list