RFR(XL): 8199712: Flight Recorder

[Vladimir Kozlov]

-lkstat was also used to determine SPARC CPU features as I remember.

Vladimir

On 4/26/18 9:03 AM, Erik Joelsson wrote:
> Should linking to the -lkstat library be conditional on jfr feature 
> being set, or perhaps it doesn't matter that much. In that case, adding 
> the library could be done in JvmFeatures.gmk instead.
> 
> /Erik
> 
> 
> On 2018-04-26 02:45, Magnus Ihse Bursie wrote:
>>
>> 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.
>>
>> * make/copy/Copy-jdk.jfr.gmk:
>> COPY_HOTSPOT_TRACE_FILES should be renamed to something like 
>> COPY_JFR_METADATA.
>>
>> 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)
>>
>> * 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"
>>
>> 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
>>
>