changeset 23211:86780d6ac391

isolated use of SortedMap in Graal option processing to option printing only; use j.u.Properties to load Graal options from a file (GRAAL-1371)
author Doug Simon <doug.simon@oracle.com>
date Tue, 22 Dec 2015 22:10:52 +0100
parents 942a54aadb47
children 2643ba182e6f
files graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/CompileTheWorld.java graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/HotSpotGraalCompilerFactory.java graal/com.oracle.graal.options/src/com/oracle/graal/options/OptionType.java graal/com.oracle.graal.options/src/com/oracle/graal/options/OptionValue.java graal/com.oracle.graal.options/src/com/oracle/graal/options/OptionsLoader.java graal/com.oracle.graal.options/src/com/oracle/graal/options/OptionsParser.java
diffstat 6 files changed, 49 insertions(+), 94 deletions(-) [+]
line wrap: on
line diff
--- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/CompileTheWorld.java	Tue Dec 22 01:36:21 2015 +0100
+++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/CompileTheWorld.java	Tue Dec 22 22:10:52 2015 +0100
@@ -53,6 +53,7 @@
 import java.util.Enumeration;
 import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Future;
 import java.util.concurrent.LinkedBlockingQueue;
@@ -125,19 +126,17 @@
          */
         public Config(String options) {
             if (options != null) {
-                List<String> optionSettings = new ArrayList<>();
+                Map<String, String> optionSettings = new HashMap<>();
                 for (String optionSetting : options.split("\\s+|#")) {
                     if (optionSetting.charAt(0) == '-') {
-                        optionSettings.add(optionSetting.substring(1));
-                        optionSettings.add("false");
+                        optionSettings.put(optionSetting.substring(1), "false");
                     } else if (optionSetting.charAt(0) == '+') {
-                        optionSettings.add(optionSetting.substring(1));
-                        optionSettings.add("true");
+                        optionSettings.put(optionSetting.substring(1), "true");
                     } else {
                         OptionsParser.parseOptionSettingTo(optionSetting, optionSettings);
                     }
                 }
-                OptionsParser.parseOptions(optionSettings.toArray(new String[optionSettings.size()]), this, null, null);
+                OptionsParser.parseOptions(optionSettings, this, null, null);
             }
         }
 
--- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/HotSpotGraalCompilerFactory.java	Tue Dec 22 01:36:21 2015 +0100
+++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/HotSpotGraalCompilerFactory.java	Tue Dec 22 22:10:52 2015 +0100
@@ -24,13 +24,11 @@
 
 import static jdk.vm.ci.inittimer.InitTimer.timer;
 
-import java.io.BufferedReader;
 import java.io.File;
 import java.io.FileReader;
 import java.io.IOException;
 import java.lang.reflect.Field;
-import java.util.ArrayList;
-import java.util.List;
+import java.util.HashMap;
 import java.util.Map;
 import java.util.Properties;
 
@@ -84,7 +82,9 @@
     /**
      * Parses the options in the file denoted by the {@linkplain VM#getSavedProperty(String) saved}
      * system property named {@code "graal.options.file"} if the file exists followed by the options
-     * encoded in saved system properties whose names start with {@code "graal.option."}.
+     * encoded in saved system properties whose names start with {@code "graal.option."}. Key/value
+     * pairs are parsed from the file denoted by {@code "graal.options.file"} with
+     * {@link Properties#load(java.io.Reader)}.
      */
     @SuppressWarnings("try")
     private static void initializeOptions() {
@@ -97,22 +97,15 @@
             if (optionsFile != null) {
                 File graalOptions = new File(optionsFile);
                 if (graalOptions.exists()) {
-                    try (BufferedReader br = new BufferedReader(new FileReader(graalOptions))) {
-                        String optionSetting = null;
-                        int lineNo = 1;
-                        List<String> optionSettings = new ArrayList<>();
-                        while ((optionSetting = br.readLine()) != null) {
-                            if (!optionSetting.isEmpty() && optionSetting.charAt(0) != '#') {
-                                try {
-                                    OptionsParser.parseOptionSettingTo(optionSetting, optionSettings);
-                                } catch (Throwable e) {
-                                    throw new InternalError("Error parsing " + graalOptions + ", line " + lineNo, e);
-                                }
-                            }
-                            lineNo++;
+                    try (FileReader fr = new FileReader(graalOptions)) {
+                        Properties props = new Properties();
+                        props.load(fr);
+                        Map<String, String> optionSettings = new HashMap<>();
+                        for (Map.Entry<Object, Object> e : props.entrySet()) {
+                            optionSettings.put((String) e.getKey(), (String) e.getValue());
                         }
                         try {
-                            OptionsParser.parseOptions(optionSettings.toArray(new String[optionSettings.size()]), null, odp, null);
+                            OptionsParser.parseOptions(optionSettings, null, odp, null);
                         } catch (Throwable e) {
                             throw new InternalError("Error parsing an option from " + graalOptions, e);
                         }
@@ -124,17 +117,16 @@
 
             Properties savedProps = getSavedProperties();
 
-            List<String> optionSettings = new ArrayList<>();
+            Map<String, String> optionSettings = new HashMap<>();
             for (Map.Entry<Object, Object> e : savedProps.entrySet()) {
                 String name = (String) e.getKey();
                 if (name.startsWith("graal.option.")) {
                     String value = (String) e.getValue();
-                    optionSettings.add(name.substring("graal.option.".length()));
-                    optionSettings.add(value);
+                    optionSettings.put(name.substring("graal.option.".length()), value);
                 }
             }
 
-            OptionsParser.parseOptions(optionSettings.toArray(new String[optionSettings.size()]), null, odp, null);
+            OptionsParser.parseOptions(optionSettings, null, odp, null);
         }
     }
 
--- a/graal/com.oracle.graal.options/src/com/oracle/graal/options/OptionType.java	Tue Dec 22 01:36:21 2015 +0100
+++ b/graal/com.oracle.graal.options/src/com/oracle/graal/options/OptionType.java	Tue Dec 22 22:10:52 2015 +0100
@@ -23,7 +23,7 @@
 package com.oracle.graal.options;
 
 /**
- * Classifies JVMCI options in several categories depending on who this option is relevant for.
+ * Classifies Graal options in several categories depending on who this option is relevant for.
  *
  */
 public enum OptionType {
--- a/graal/com.oracle.graal.options/src/com/oracle/graal/options/OptionValue.java	Tue Dec 22 01:36:21 2015 +0100
+++ b/graal/com.oracle.graal.options/src/com/oracle/graal/options/OptionValue.java	Tue Dec 22 22:10:52 2015 +0100
@@ -156,7 +156,7 @@
     private OptionValue<?> next;
     private static OptionValue<?> head;
 
-    private static final boolean ShowReadsHistogram = Boolean.getBoolean("jvmci.showOptionValueReadsHistogram");
+    private static final boolean ShowReadsHistogram = Boolean.getBoolean("graal.showOptionValueReadsHistogram");
 
     private static void addToHistogram(OptionValue<?> option) {
         if (ShowReadsHistogram) {
--- a/graal/com.oracle.graal.options/src/com/oracle/graal/options/OptionsLoader.java	Tue Dec 22 01:36:21 2015 +0100
+++ b/graal/com.oracle.graal.options/src/com/oracle/graal/options/OptionsLoader.java	Tue Dec 22 22:10:52 2015 +0100
@@ -22,15 +22,15 @@
  */
 package com.oracle.graal.options;
 
+import java.util.HashMap;
+import java.util.Map;
 import java.util.ServiceLoader;
-import java.util.SortedMap;
-import java.util.TreeMap;
 
 /**
  * Helper class used to load option descriptors. Only to be used in the slow-path.
  */
 public class OptionsLoader {
-    public static final SortedMap<String, OptionDescriptor> options = new TreeMap<>();
+    public static final Map<String, OptionDescriptor> options = new HashMap<>();
 
     /**
      * Initializes {@link #options} from {@link Options} services.
--- a/graal/com.oracle.graal.options/src/com/oracle/graal/options/OptionsParser.java	Tue Dec 22 01:36:21 2015 +0100
+++ b/graal/com.oracle.graal.options/src/com/oracle/graal/options/OptionsParser.java	Tue Dec 22 22:10:52 2015 +0100
@@ -26,16 +26,16 @@
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Formatter;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.ServiceLoader;
 import java.util.Set;
 import java.util.SortedMap;
+import java.util.TreeMap;
 
 /**
- * This class contains methods for parsing JVMCI options and matching them against a set of
- * {@link OptionDescriptors}. The {@link OptionDescriptors} are loaded from JVMCI jars, either
+ * This class contains methods for parsing Graal options and matching them against a set of
+ * {@link OptionDescriptors}. The {@link OptionDescriptors} are loaded from Graal jars, either
  * {@linkplain GraalJarsOptionDescriptorsProvider directly} or via a {@link ServiceLoader}.
  */
 public class OptionsParser {
@@ -59,33 +59,23 @@
     }
 
     /**
-     * Parses an ordered list of (name, value) pairs assigning values to JVMCI options.
+     * Parses a map representing assignments of values to options.
      *
-     * @param optionSettings JVMCI options as serialized (name, value) pairs
+     * @param optionSettings option settings (i.e., assignments of values to options)
      * @param setter the object to notify of the parsed option and value
      * @param odp if non-null, the service to use for looking up {@link OptionDescriptor}s
      * @param options the options database to use if {@code odp == null}. If
      *            {@code options == null && odp == null}, {@link OptionsLoader#options} is used.
      * @throws IllegalArgumentException if there's a problem parsing {@code option}
      */
-    public static void parseOptions(String[] optionSettings, OptionConsumer setter, OptionDescriptorsProvider odp, SortedMap<String, OptionDescriptor> options) {
-        if (optionSettings != null && optionSettings.length != 0) {
-            assert optionSettings.length % 2 == 0;
+    public static void parseOptions(Map<String, String> optionSettings, OptionConsumer setter, OptionDescriptorsProvider odp, Map<String, OptionDescriptor> options) {
+        if (optionSettings != null && !optionSettings.isEmpty()) {
 
-            moveHelpFlagsToTail(optionSettings);
-
-            for (int i = 0; i < optionSettings.length / 2; i++) {
-                String name = optionSettings[i * 2];
-                String value = optionSettings[i * 2 + 1];
-                parseOption(name, value, setter, odp, options);
+            for (Map.Entry<String, String> e : optionSettings.entrySet()) {
+                parseOption(e.getKey(), e.getValue(), setter, odp, options);
             }
             if (PrintFlags.getValue() || ShowFlags.getValue()) {
-                Set<String> explicitlyAssigned = new HashSet<>(optionSettings.length / 2);
-                for (int i = 0; i < optionSettings.length / 2; i++) {
-                    String name = optionSettings[i * 2];
-                    explicitlyAssigned.add(name);
-                }
-                printFlags(resolveOptions(options), "JVMCI", System.out, explicitlyAssigned);
+                printFlags(resolveOptions(options), "Graal", System.out, optionSettings.keySet());
                 if (PrintFlags.getValue()) {
                     System.exit(0);
                 }
@@ -94,54 +84,23 @@
     }
 
     /**
-     * Moves all {@code PrintFlags} and {@code ShowFlags} option settings to the back of
-     * {@code optionSettings}. This allows the help message to show which options had their value
-     * explicitly set (even if to their default value).
-     */
-    private static void moveHelpFlagsToTail(String[] optionSettings) {
-        List<String> tail = null;
-        int insert = 0;
-        for (int i = 0; i < optionSettings.length / 2; i++) {
-            String name = optionSettings[i * 2];
-            String value = optionSettings[i * 2 + 1];
-            if (name.equals("ShowFlags") || name.equals("PrintFlags")) {
-                if (tail == null) {
-                    tail = new ArrayList<>(4);
-                    insert = i * 2;
-                }
-                tail.add(name);
-                tail.add(value);
-            } else if (tail != null) {
-                optionSettings[insert++] = name;
-                optionSettings[insert++] = value;
-            }
-        }
-        if (tail != null) {
-            assert tail.size() + insert == optionSettings.length;
-            String[] tailArr = tail.toArray(new String[tail.size()]);
-            System.arraycopy(tailArr, 0, optionSettings, insert, tailArr.length);
-        }
-    }
-
-    /**
-     * Parses a given option setting string to a list of (name, value) pairs.
+     * Parses a given option setting string to a map of settings.
      *
      * @param optionSetting a string matching the pattern {@code <name>=<value>}
      */
-    public static void parseOptionSettingTo(String optionSetting, List<String> dst) {
+    public static void parseOptionSettingTo(String optionSetting, Map<String, String> dst) {
         int eqIndex = optionSetting.indexOf('=');
         if (eqIndex == -1) {
             throw new InternalError("Option setting has does not match the pattern <name>=<value>: " + optionSetting);
         }
-        dst.add(optionSetting.substring(0, eqIndex));
-        dst.add(optionSetting.substring(eqIndex + 1));
+        dst.put(optionSetting.substring(0, eqIndex), optionSetting.substring(eqIndex + 1));
     }
 
     /**
      * Resolves {@code options} to a non-null value. This ensures {@link OptionsLoader#options} is
      * only loaded if necessary.
      */
-    private static SortedMap<String, OptionDescriptor> resolveOptions(SortedMap<String, OptionDescriptor> options) {
+    private static Map<String, OptionDescriptor> resolveOptions(Map<String, OptionDescriptor> options) {
         return options != null ? options : OptionsLoader.options;
     }
 
@@ -156,14 +115,14 @@
      *            {@code options == null && odp == null}, {@link OptionsLoader#options} is used.
      * @throws IllegalArgumentException if there's a problem parsing {@code option}
      */
-    private static void parseOption(String name, String valueString, OptionConsumer setter, OptionDescriptorsProvider odp, SortedMap<String, OptionDescriptor> options) {
+    private static void parseOption(String name, String valueString, OptionConsumer setter, OptionDescriptorsProvider odp, Map<String, OptionDescriptor> options) {
 
         OptionDescriptor desc = odp != null ? odp.get(name) : resolveOptions(options).get(name);
         if (desc == null) {
             if (name.equals("PrintFlags")) {
-                desc = OptionDescriptor.create("PrintFlags", Boolean.class, "Prints all JVMCI flags and exits", OptionsParser.class, "PrintFlags", PrintFlags);
+                desc = OptionDescriptor.create("PrintFlags", Boolean.class, "Prints all Graal flags and exits", OptionsParser.class, "PrintFlags", PrintFlags);
             } else if (name.equals("ShowFlags")) {
-                desc = OptionDescriptor.create("ShowFlags", Boolean.class, "Prints all JVMCI flags and continues", OptionsParser.class, "ShowFlags", ShowFlags);
+                desc = OptionDescriptor.create("ShowFlags", Boolean.class, "Prints all Graal flags and continues", OptionsParser.class, "ShowFlags", ShowFlags);
             }
         }
         if (desc == null) {
@@ -272,10 +231,15 @@
         return lines;
     }
 
-    private static void printFlags(SortedMap<String, OptionDescriptor> sortedOptions, String prefix, PrintStream out, Set<String> explicitlyAssigned) {
+    private static void printFlags(Map<String, OptionDescriptor> options, String prefix, PrintStream out, Set<String> explicitlyAssigned) {
+        SortedMap<String, OptionDescriptor> sortedOptions;
+        if (options instanceof SortedMap) {
+            sortedOptions = (SortedMap<String, OptionDescriptor>) options;
+        } else {
+            sortedOptions = new TreeMap<>(options);
+        }
         out.println("[List of " + prefix + " options]");
         for (Map.Entry<String, OptionDescriptor> e : sortedOptions.entrySet()) {
-            e.getKey();
             OptionDescriptor desc = e.getValue();
             Object value = desc.getOptionValue().getValue();
             List<String> helpLines = wrap(desc.getHelp(), 70);
@@ -311,7 +275,7 @@
     /**
      * Returns the set of options that fuzzy match a given option name.
      */
-    private static List<OptionDescriptor> fuzzyMatch(SortedMap<String, OptionDescriptor> options, String optionName) {
+    private static List<OptionDescriptor> fuzzyMatch(Map<String, OptionDescriptor> options, String optionName) {
         List<OptionDescriptor> matches = new ArrayList<>();
         for (Map.Entry<String, OptionDescriptor> e : options.entrySet()) {
             float score = stringSimiliarity(e.getKey(), optionName);