Uploaded image for project: 'Java Mission Control'
  1. Java Mission Control
  2. JMC-6075

Avoid conversion for RECORDING_SETTING event from JDK9/10 in SettingsTransformer

    Details

      Description

      Need to address the following review comments from JMC-5895.

      Jay's Comments:
      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.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.

      Marcus's Comments:

      To make the difference between v1 and v2 more maintainable and clear, we should probably have v1 and v2 being their own parsers with (plenty of) shared infrastructure. That said, since we need to get an EA out soon, I think that can wait.

      I think it may be a good idea to have a Jdk9TypeIDs.java, at least for a subset of the type IDs (like RECORDING_SETTING), but since no one should be using it, except our internal machinery, it should be package visible only.

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                jdv Jayathirth D V
                Reporter:
                sballal Sharath Ballal (Inactive)
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: