[11u] RFR: JDK-8205516: JFR tool

[Mario Torre]

Hi Jie,

>From what I can see the patch is good to be pushed, but we need a
maintainer labelling JDK-8214896 and JDK-8214925 for approval.

Cheers,
Mario

On Mon, Aug 12, 2019 at 8:59 PM Jie Kang <jkang at redhat.com> wrote:
>
> Hi all,
>
> Please review this jdk11u backport. The patch did *not* apply cleanly
> and has changes described following the links below.  A CSR was opened
> and approved for this backport. As well Oracle has brought this for
> 11.0.6 so we should not push this until 11.0.6 opens (Aug 27).
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8205516
> Webrev: http://cr.openjdk.java.net/~jkang/8205516/webrev.01/
> CSR: https://bugs.openjdk.java.net/browse/JDK-8228670
>
> * Addition of changes to PrettyWriter and RecordedObject from JDK-8165675 [1]
>
> PrettyWriter is newly added with this backport. Since it did not exist
> at the time for the JDK-8165675 backport, the changes there for
> PrettyWriter were not included. These are included within this patch.
> As well some lines in the JDK-8165675 backport were skipped for
> RecordedObject causing a test to fail, and are now included so the
> test passes. Let me know if there is a different process for this
> situation!
>
> [1]
> https://bugs.openjdk.java.net/browse/JDK-8165675
> PrettyWriter changes:
> http://hg.openjdk.java.net/jdk/jdk/rev/4eff16f47ae2#l5.2
> RecordedObject changes:
> http://hg.openjdk.java.net/jdk/jdk/rev/4eff16f47ae2#l4.35
>
> * Updates to test/jdk/jdk/jfr/api/metadata/annotations/TestFormatMissingValue.java
>
> This test is updated to match this backport. There are comments in the
> changeset visible that describe why.
>
> * A small update to
> src/jdk.jfr/share/classes/jdk/jfr/internal/tool/PrettyWriter.java
>
>          if (dataAmount != null) {
>              if (value instanceof Number) {
>                  Number n = (Number) value;
> -                String bytes = Utils.formatBytes(n.longValue(), " ");
> +                String bytes = Utils.formatBytes(n.longValue());
>                  if (field.getAnnotation(Frequency.class) != null) {
>                      bytes += "/s";
>                  }
>
> This is to match the function definition of Utils.formatBytes in
> jdk11u-dev. This is actually also what is in jdk/jdk, but it is a
> change here as the order of commits in jdk11u-dev differ from jdk/jdk.
>
>
> Also. this backport should be integrated with the two follow-up bug
> fixes at the same time to maintain a clean working repository
> (compiles and tests pass). Both cleanly apply after my patch. I have
> added fix request label and comment to them.
>
> JDK-8214896: JFR Tool left files behind
> https://bugs.openjdk.java.net/browse/JDK-8214896
>
> JDK-8214925: [TESTBUG] JFR tool fails to execute
> https://bugs.openjdk.java.net/browse/JDK-8214925
>
> I have run the tier one tests, jdk.jfr tests and some manual tests of
> the jfr tool binary with all three fixes applied together, but third
> party verification would be greatly appreciated. Let me know what you
> think!
>
>
> Regards,



-- 
Mario Torre
Associate Manager, Software Engineering
Red Hat GmbH <https://www.redhat.com>
9704 A60C B4BE A8B8 0F30  9205 5D7E 4952 3F65 7898