RFR(XL): 8199712: Flight Recorder

[Erik Gahlin]

Hi Magnus,

Sorry about not including build-dev.  See comments inline.

>
> On 2018-04-25 13:06, Erik Gahlin wrote:
>> Greetings,
>>
>> Could I have a review of 8199712: Flight Recorder
>>
>> As mentioned in the preview [1] the tracing backend has been removed. 
>> Event metadata has been consolidated into a single XML file and event 
>> classes are now generated by GenerateJfrFiles.java.
>>
>> Tests have been run on Linux-x64, Windows-x64 and MaxOSX-x64.
>>
>> For details about the feature, see the JEP:
>> https://bugs.openjdk.java.net/browse/JDK-8193393
>>
>> Webrev:
>> http://cr.openjdk.java.net/~egahlin/8199712.0/
> Hi Erik,
>
> When posting build changes (files in "make/*"), please always include 
> build-dev. Thanks! :-)
>
> This review is for build changes only.
>
> * make/RunTestsPrebuiltSpec.gmk: This file has only a copyright year 
> change. It can be reverted.
>
Will fix.

> * make/copy/Copy-jdk.jfr.gmk:
> COPY_HOTSPOT_TRACE_FILES should be renamed to something like 
> COPY_JFR_METADATA.
>
Will fix.

> The second part is better off rewritten using SetupCopyFiles.
>
> Something like this, written on-the-fly, so might not work. (Email me 
> privately if it does not work and you need help.)
>
> JFR_CONF_DIR := $(TOPDIR)/src/jdk.jfr/share/conf/jfr
>
> $(eval $(call SetupCopyFiles, COPY_JFR_CONF, \
>   DEST := $(LIB_DST_DIR)/jfr, \
>   FILES := $(wildcard $(JFR_CONF_DIR)/*.jfc), \
>   FLATTEN := true, \
> ))
>
> TARGETS += $(COPY_JFR_CONF)
>
Will look into it.

> * make/autoconf/hotspot.m4:
> It's a bit unfortunate with the --enable-jfr option. Ideally, this 
> should be handled only as a JVM feature (--with-jvm-features=jfr). Try 
> this piece of code instead: (Not tested, but should work.)
>
> diff --git a/make/autoconf/hotspot.m4 b/make/autoconf/hotspot.m4
> --- a/make/autoconf/hotspot.m4
> +++ b/make/autoconf/hotspot.m4
> @@ -26,7 +26,7 @@
>  # All valid JVM features, regardless of platform
>  VALID_JVM_FEATURES="compiler1 compiler2 zero minimal dtrace jvmti 
> jvmci \
>      graal vm-structs jni-check services management all-gcs nmt cds \
> -    static-build link-time-opt aot"
> +    static-build link-time-opt aot jfr"
>
>  # All valid JVM variants
>  VALID_JVM_VARIANTS="server client minimal core zero custom"
> @@ -313,6 +313,11 @@
>      AC_MSG_ERROR([Specified JVM feature 'vm-structs' requires feature 
> 'all-gcs'])
>    fi
>
> +  # Enable JFR by default, except on linux-sparcv9 and on minimal.
> +  if test "x$OPENJDK_TARGET_OS" != xlinux || test 
> "x$OPENJDK_TARGET_CPU" != xsparcv9; then
> +    NON_MINIMAL_FEATURES="$NON_MINIMAL_FEATURES jfr"
> +  fi
> +
>    # Turn on additional features based on other parts of configure
>    if test "x$INCLUDE_DTRACE" = "xtrue"; then
>      JVM_FEATURES="$JVM_FEATURES dtrace"
>
Will try it.

Thanks
Erik

> Otherwise, build changes look good!
>
> /Magnus
>
>
>>
>> Bug:
>> https://bugs.openjdk.java.net/browse/JDK-8199712
>>
>> [1] 
>> http://mail.openjdk.java.net/pipermail/hotspot-dev/2018-April/031359.html
>>
>> Thanks
>> Erik and Markus
>