Re: RFR: JDK-8191324: SA cleanup -- part 2

[serguei.spit...@oracle.com]

Hi Jini, It looks good to me. A couple of questions on the:   http://cr.openjdk.java.net/~jgeorge/8191324/webrev.00/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/runtime/PerfDataEntry.java.udiff.html + private static int U_None; + private static int U_Bytes; + private static int U_Ticks; + private static int U_Events; + private static int U_String; + private static int U_Hertz; + . . . + + // PerfData Units enum + U_None = db.lookupIntConstant("PerfData::U_None"); + U_Bytes = db.lookupIntConstant("PerfData::U_Bytes"); + U_Ticks = db.lookupIntConstant("PerfData::U_Ticks"); + U_Events = db.lookupIntConstant("PerfData::U_Events"); + U_String = db.lookupIntConstant("PerfData::U_String"); + U_Hertz = db.lookupIntConstant("PerfData::U_Hertz"); Before your fix these private static fields were defined in the interface which I like: - public interface PerfDataUnits { - public static final int U_None = 1; - public static final int U_Bytes = 2; - public static final int U_Ticks = 3; - public static final int U_Events = 4; - public static final int U_String = 5; - public static final int U_Hertz = 6; - } I think, it'd still make sense to keep them in an interface or a small class, so they are not separate constants but a part of a common outer type. - public interface PerfDataVariability { - public static final int V_Constant = 1; - public static final int V_Monotonic = 2; - public static final int V_Variable = 3; - } I wonder it these constants can still be useful as the following method returns one of them as a result which may need to be checked for an exact value. Thanks, Serguei On 11/21/17 02:34, Jini George wrote: Hello, Here's requesting reviews for some SA code cleanup. ID: https://bugs.openjdk.java.net/browse/JDK-8191324 Webrev: http://cr.openjdk.java.net/~jgeorge/8191324/webrev.00/index.html The changes here are primarily to: 1. Remove unused IA64 SA code. 2. Make changes to avoid error-prone redefinition of hotspot constants in SA Java code. Instead read it in through vmStructs and db.lookupIntConstant(). 3. Make variable name changes in SA to align with the hotspot names. The changes are straightforward. Thanks much, Jini.