JMC-5698: Improving Duplicate Flags rule

Ken Dobson kdobson at redhat.com
Thu Nov 8 19:49:53 UTC 2018


Hi all,

This is a patch to improve the output of the Duplicate Flags rule to be
more clear. I've attached a screenshot showing the new output. Please
review it and let me know if you have any comments.

https://bugs.openjdk.java.net/projects/JMC/issues/JMC-5698

Thanks,

Ken Dobson

diff -r b3a23786ef23
core/org.openjdk.jmc.flightrecorder.rules.jdk/src/main/java/org/openjdk/jmc/flightrecorder/rules/jdk/dataproviders/JvmInternalsDataProvider.java
---
a/core/org.openjdk.jmc.flightrecorder.rules.jdk/src/main/java/org/openjdk/jmc/flightrecorder/rules/jdk/dataproviders/JvmInternalsDataProvider.java
Wed Oct 10 16:43:35 2018 -0400
+++
b/core/org.openjdk.jmc.flightrecorder.rules.jdk/src/main/java/org/openjdk/jmc/flightrecorder/rules/jdk/dataproviders/JvmInternalsDataProvider.java
Thu Nov 08 14:31:12 2018 -0500
@@ -32,10 +32,10 @@
  */
 package org.openjdk.jmc.flightrecorder.rules.jdk.dataproviders;

+import java.util.ArrayList;
+import java.util.Collection;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.Map;
-import java.util.Set;

 /**
  * Helper class used to share analysis of JVM related information, such as
flags.
@@ -43,33 +43,33 @@
 @SuppressWarnings("nls")
 public class JvmInternalsDataProvider {

-    private static final String[] PREFIXES = new String[] {"Xmx", "Xms",
"Xmn", "Xss", "Xmaxjitcodesize"};
+    private static final String[] PREFIXES = new String[] {"-Xmx", "-Xms",
"-Xmn", "-Xss", "-Xmaxjitcodesize"};
     /**
      * Options that are OK to use multiple times if different values are
provided. Check for
      * duplicates using the full argument.
      */
-    private static final String[] VERBATIM = new String[] {"verbose",
"-add-exports"};
+    private static final String[] VERBATIM = new String[] {"-verbose",
"--add-exports", "--add-opens"};
     private static final Map<String, String> EQUIVALENT = new HashMap<>();

     static {
-        putBiMap("Xbatch", "BackgroundCompilation");
-        putBiMap("Xmaxjitcodesize", "ReservedCodeCacheSize");
-        putBiMap("Xmx", "MaxHeapSize");
-        putBiMap("Xmn", "NewSize");
-        putBiMap("Xss", "ThreadStackSize");
-        putBiMap("Xusealtsigs", "UseAltSigs");
-        putBiMap("cp", "classpath");
-        putBiMap("esa", "enablesystemassertions");
-        putBiMap("dsa", "disablesystemassertions");
-        putBiMap("Xconcgc", "UseConcMarkSweepGC");
-        putBiMap("Xnoconcgc", "UseConcMarkSweepGC");
-        putBiMap("Xnoclassgc", "ClassUnloading");
-        putBiMap("Xminf", "MinHeapFreeRatio");
-        putBiMap("Xmaxf", "MaxHeapFreeRatio");
-        putBiMap("Xrs", "ReduceSignalUsage");
-        putBiMap("Dcom.sun.management", "ManagementServer");
-        putBiMap("Xshare:dump", "DumpSharedSpaces");
-        putBiMap("Xboundthreads", "UseBoundThreads");
+        putBiMap("-Xbatch", "BackgroundCompilation");
+        putBiMap("-Xmaxjitcodesize", "ReservedCodeCacheSize");
+        putBiMap("-Xmx", "MaxHeapSize");
+        putBiMap("-Xmn", "NewSize");
+        putBiMap("-Xss", "ThreadStackSize");
+        putBiMap("-Xusealtsigs", "UseAltSigs");
+        putBiMap("-cp", "classpath");
+        putBiMap("-esa", "enablesystemassertions");
+        putBiMap("-dsa", "disablesystemassertions");
+        putBiMap("-Xconcgc", "UseConcMarkSweepGC");
+        putBiMap("-Xnoconcgc", "UseConcMarkSweepGC");
+        putBiMap("-Xnoclassgc", "ClassUnloading");
+        putBiMap("-Xminf", "MinHeapFreeRatio");
+        putBiMap("-Xmaxf", "MaxHeapFreeRatio");
+        putBiMap("-Xrs", "ReduceSignalUsage");
+        putBiMap("-Dcom.sun.management", "ManagementServer");
+        putBiMap("-Xshare:dump", "DumpSharedSpaces");
+        putBiMap("-Xboundthreads", "UseBoundThreads");
         putBiMap("AlwaysTenure", "NeverTenure");
         putBiMap("ResizeTLE", "ResizeTLAB");
         putBiMap("PrintTLE", "PrintTLAB");
@@ -77,10 +77,10 @@
         putBiMap("UseTLE", "UseTLAB");
         putBiMap("UsePermISM", "UseISM");
         putBiMap("G1MarkStackSize", "CMSMarkStackSize");
-        putBiMap("Xms", "InitialHeapSize");
+        putBiMap("-Xms", "InitialHeapSize");
         putBiMap("DisplayVMOutputToStderr", "DisplayVMOutputToStdout");
-        putBiMap("Xverify", "BytecodeVerificationLocal");
-        putBiMap("Xverify", "BytecodeVerificationRemote");
+        putBiMap("-Xverify", "BytecodeVerificationLocal");
+        putBiMap("-Xverify", "BytecodeVerificationRemote");
         putBiMap("DefaultMaxRAMFraction", "MaxRAMFraction");
         putBiMap("CMSMarkStackSizeMax", "MarkStackSizeMax");
         putBiMap("ParallelMarkingThreads", "ConcGCThreads");
@@ -100,14 +100,13 @@
      *            the set of JVM flags to check
      * @return a set of all duplicated JVM flags
      */
-    public static Set<String> checkDuplicates(String arguments) {
-        HashSet<String> seenFlags = new HashSet<>();
-        HashSet<String> dupes = new HashSet<>();
-        String[] argumentArray = arguments.split(" -");
+    public static Collection<ArrayList<String>> checkDuplicates(String
arguments) {
+        HashMap<String, String> seenFlags = new HashMap<>();
+        HashMap<String, ArrayList<String>> dupes = new HashMap<>();
+        String[] argumentArray = arguments.split(" ");
         if (argumentArray.length == 1 && argumentArray[0].equals("")) {
-            return dupes;
+            return dupes.values();
         }
-        argumentArray[0] = argumentArray[0].substring(1);
         for (String fullArgument : argumentArray) {
             boolean verbatim = false;
             for (int i = 0; i < VERBATIM.length; i++) {
@@ -122,7 +121,7 @@
             } else {
                 String[] split = fullArgument.split("[=:]", 3);
                 argument = split[0];
-                if ("XX".equals(split[0])) {
+                if ("-XX".equals(split[0])) {
                     argument = split[1];
                     if (argument.startsWith("+") ||
argument.startsWith("-")) {
                         argument = argument.substring(1);
@@ -132,22 +131,24 @@
                     argument = scrubPrefix(argument, PREFIXES[i]);
                 }
                 String equivalentArgument = EQUIVALENT.get(argument);
-                if (equivalentArgument != null &&
!seenFlags.contains(argument)
-                        && seenFlags.contains(equivalentArgument)) {
-                    String longerArgument = equivalentArgument.length() >
argument.length() ? equivalentArgument
-                            : argument;
-                    String shorterArgument = equivalentArgument.length() >
argument.length() ? argument
-                            : equivalentArgument;
-                    String combinedArgument = String.format("%s (%s)",
longerArgument, shorterArgument);
-                    dupes.add(combinedArgument);
+                if (equivalentArgument != null &&
!seenFlags.containsKey(argument)
+                        && seenFlags.containsKey(equivalentArgument)) {
+                    argument = equivalentArgument;
                 }
             }
-            if (seenFlags.contains(argument)) {
-                dupes.add(argument);
+            if (seenFlags.containsKey(argument)) {
+                if (!dupes.containsKey(argument)) {
+                    dupes.put(argument, new ArrayList<String>());
+                    dupes.get(argument).add(seenFlags.get(argument));
+                }
+                dupes.get(argument).add(fullArgument);
+
             }
-            seenFlags.add(argument);
+            else {
+                seenFlags.put(argument, fullArgument);
+            }
         }
-        return dupes;
+        return dupes.values();
     }

     private static String scrubPrefix(String argument, String prefix) {
diff -r b3a23786ef23
core/org.openjdk.jmc.flightrecorder.rules.jdk/src/main/java/org/openjdk/jmc/flightrecorder/rules/jdk/general/DuplicateFlagsRule.java
---
a/core/org.openjdk.jmc.flightrecorder.rules.jdk/src/main/java/org/openjdk/jmc/flightrecorder/rules/jdk/general/DuplicateFlagsRule.java
Wed Oct 10 16:43:35 2018 -0400
+++
b/core/org.openjdk.jmc.flightrecorder.rules.jdk/src/main/java/org/openjdk/jmc/flightrecorder/rules/jdk/general/DuplicateFlagsRule.java
Thu Nov 08 14:31:12 2018 -0500
@@ -33,6 +33,7 @@
 package org.openjdk.jmc.flightrecorder.rules.jdk.general;

 import java.text.MessageFormat;
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Set;
@@ -72,14 +73,15 @@
         // FIXME: Should we check if there are different jvm args in
different chunks?
         Set<String> args =
jvmInfoItems.getAggregate(Aggregators.distinct(JdkAttributes.JVM_ARGUMENTS));
         if (args != null && !args.isEmpty()) {
-
-            Set<String> dupes =
JvmInternalsDataProvider.checkDuplicates(args.iterator().next());
+
+            Collection<ArrayList<String>> dupes = JvmInternalsDataProvider.
+                    checkDuplicates(args.iterator().next());

             if (!dupes.isEmpty()) {
                 StringBuilder sb = new StringBuilder();
                 sb.append("<ul>"); //$NON-NLS-1$
-                for (String dupe : dupes) {
-                    sb.append("<li>" + Encode.forHtml(dupe) + "</li>");
//$NON-NLS-1$ //$NON-NLS-2$
+                for (ArrayList<String> dupe : dupes) {
+                    sb.append("<li>" + Encode.forHtml(String.join(", ",
dupe)) + "</li>"); //$NON-NLS-1$ //$NON-NLS-2$
                 }
                 sb.append("</ul>"); //$NON-NLS-1$
                 String shortDescription = dupes.size() > 1


More information about the jmc-dev mailing list