RFR: JMC-5895 - JFR in JDK 11 changed event paths from 'com.oracle.jdk' to 'jdk', and type paths from 'com.oracle.jfr.types' to 'jdk.types'

[Jayathirth D V]

Hi Sharath,

Please find my observations:

After webrev.02 , For each event in the code to check whether we have Event from JDK 9 or 10 we are using JdkTypeIDsPreJdk11.needtransform() function. This might make automated analysis take more time. We can completely remove the usage of this verification if we maintain JDK 9/10 Event types in a file like JdkTypeIDs.java. We can have new file like Jdk9TypeIDs.java with events starting from "com.oracle.jdk.".

After doing above thing we can make following changes in other files:

In SettingsTransformer.java:
	In wrapSinkFactory.create() we can use:
	if (JdkTypeIDsPreJdk11.RECORDING_SETTING.equals(identifier) ||
	     Jdk9TypeIDs.RECORDING_SETTING.equals(identifier)) {

	Both these conditions will take care of whether event is from JDK 7/8 & 9/10. We can remove needtransform() check while validating created SettingsTransformer object.

	In SettingsTransformer() constructor, for Jdk 9/10 events looks like we are not able to find any field which matches END_TIME. So we are creating EventSink with all the data structures. 
	In SettingsTransformer.addEvent() we can remove the check for JdkTypeIDsPreJdk11.needTransform() because we are creating SettingsTransformer only when we know we have event types from JDK 7/8/9/10.

In SyntheticAttributeExtension.java:
	If we have Jdk9TypeIDs.java there is no need to explicitly call JdkTypeIDsPreJdk11.translate().  Also here we can use single if block like:
		public String getValueInterpretation(String eventTypeId, String fieldId) {
		if (REC_SETTING_EVENT_ID_ATTRIBUTE.getIdentifier().equals(fieldId)
				&& (JdkTypeIDsPreJdk11.RECORDING_SETTING.equals(eventTypeId) ||
					Jdk9TypeIDs.RECORDING_SETTING.equals(eventTypeId) ||
					JdkTypeIDs.RECORDING_SETTING.equals(eventTypeId) {
			return JfrInternalConstants.TYPE_IDENTIFIER_VALUE_INTERPRETATION;
		}
		return null;
	}

In JdkTypeIDsPreJdk11.java:
	With above changes we can remove needtransform() function.

Generic change in TypeManager.java:
	Looks like there is no need to create new function createStructReaderV2(). We can use createStructReaderV1() by adding new "switch identifiers" as most of them are same between createStructReaderV1() & createStructReaderV2 ().

Since I am new to JMC project I wanted to verify things before mentioning my observations. So I made above changes locally and was able to build and run unit tests without failure:)

Thanks,
Jay

-----Original Message-----
From: Sharath Ballal 
Sent: Saturday, July 14, 2018 10:32 AM
To: jmc-dev at openjdk.java.net
Subject: RE: RFR: JMC-5895 - JFR in JDK 11 changed event paths from 'com.oracle.jdk' to 'jdk', and type paths from 'com.oracle.jfr.types' to 'jdk.types'

The fix has undergone changes from last time as 'mvn verify' was failing.  The code changes now include: 

1.      Changes to the V1 parser to use the SettingsTransformer for JDK 9 & 10 events.

2.      Changes to some of the tests and their baseline files.

 

Testing done:

mvn verify passes.

JDK 7/8/9/10 - Recording, loading and automated analysis works.  Verified that  scores of the automated analysis doesn't change from earlier.

JDK 11 - Recording, loading and automated analysis works.  

 

Latest webrev at : http://cr.openjdk.java.net/~sballal/JMC-5895/webrev.03/ 

Kindly review the fix.

 

Thanks,

Sharath

 

 

From: Sharath Ballal [mailto:sharath.ballal at oracle.com] 
Sent: Friday, June 08, 2018 3:03 PM
To: 'jmc-dev at openjdk.java.net'
Subject: RFR: JMC-5895 - JFR in JDK 11 changed event paths from 'com.oracle.jdk' to 'jdk', and type paths from 'com.oracle.jfr.types' to 'jdk.types'

 

Hello,

 

Pls review fix for the following P1 issue:

 

ID: https://bugs.openjdk.java.net/browse/JMC-5895 

Webrev: http://cr.openjdk.java.net/~sballal/JMC-5895/webrev.00/ 

 

Thanks,

Sharath