changeset 22601:bc2d4dac0cd3

JVMCI options are now passed as individual -Djvmci.option.<name>=<value> arguments on the command line instead of -Djvmci.options=<multiple options settings separated by spaces>
author Doug Simon <doug.simon@oracle.com>
date Tue, 22 Sep 2015 22:29:28 +0200
parents a4b63f56bc97
children d3930fdd1eb3
files jvmci/jdk.internal.jvmci.options/src/jdk/internal/jvmci/options/OptionsParser.java mx.jvmci/mx_jvmci.py src/share/vm/jvmci/jvmciRuntime.cpp src/share/vm/jvmci/jvmciRuntime.hpp src/share/vm/runtime/arguments.cpp src/share/vm/runtime/thread.cpp
diffstat 6 files changed, 130 insertions(+), 170 deletions(-) [+]
line wrap: on
line diff
--- a/jvmci/jdk.internal.jvmci.options/src/jdk/internal/jvmci/options/OptionsParser.java	Tue Sep 22 11:57:49 2015 -0700
+++ b/jvmci/jdk.internal.jvmci.options/src/jdk/internal/jvmci/options/OptionsParser.java	Tue Sep 22 22:29:28 2015 +0200
@@ -46,11 +46,6 @@
  */
 public class OptionsParser {
 
-    /**
-     * Character used to escape a space or a literal % in a JVMCI option value.
-     */
-    private static final char ESCAPE = '%';
-
     private static final OptionValue<Boolean> PrintFlags = new OptionValue<>(false);
 
     /**
@@ -69,73 +64,18 @@
     }
 
     /**
-     * Finds the index of the next character in {@code s} starting at {@code from} that is a
-     * {@linkplain #ESCAPE non-escaped} space iff {@code spaces == true}.
-     */
-    private static int skip(String s, int from, boolean spaces) {
-        int len = s.length();
-        int i = from;
-        while (i < len) {
-            char ch = s.charAt(i);
-            if (ch == ESCAPE) {
-                if (i == len - 1) {
-                    throw new InternalError("Escape character " + ESCAPE + " cannot be at end of jvmci.options value: " + s);
-                }
-                ch = s.charAt(i + 1);
-                if (ch != ESCAPE && ch != ' ') {
-                    throw new InternalError("Escape character " + ESCAPE + " must be followed by space or another " + ESCAPE + " character");
-                }
-                if (spaces) {
-                    return i;
-                }
-                i++;
-            } else if (ch == ' ' != spaces) {
-                return i;
-            }
-            i++;
-        }
-        return len;
-    }
-
-    private static String unescape(String s) {
-        int esc = s.indexOf(ESCAPE);
-        if (esc == -1) {
-            return s;
-        }
-        StringBuilder sb = new StringBuilder(s.length());
-        int start = 0;
-        do {
-            sb.append(s.substring(start, esc));
-            char escaped = s.charAt(esc + 1);
-            if (escaped == ' ') {
-                sb.append(' ');
-            } else {
-                assert escaped == ESCAPE;
-                sb.append(ESCAPE);
-            }
-            start = esc + 2;
-            esc = s.indexOf(ESCAPE, start);
-        } while (esc != -1);
-        if (start < s.length()) {
-            sb.append(s.substring(start));
-        }
-        return sb.toString();
-    }
-
-    /**
      * Parses the options in {@code <jre>/lib/jvmci/options} if {@code parseOptionsFile == true} and
-     * the file exists followed by the {@linkplain #ESCAPE non-escaped} space separated JVMCI
-     * options in {@code options} if {@code options != null}.
+     * the file exists followed by the JVMCI options in {@code options} if {@code options != null}.
      *
      * Called from VM. This method has an object return type to allow it to be called with a VM
      * utility function used to call other static initialization methods.
      *
-     * @param options {@linkplain #ESCAPE non-escaped} space separated set of JVMCI options to parse
+     * @param options JVMCI options as serialized (name, value) pairs
      * @param parseOptionsFile specifies whether to look for and parse
      *            {@code <jre>/lib/jvmci.options}
      */
     @SuppressWarnings("try")
-    public static Boolean parseOptionsFromVM(String options, boolean parseOptionsFile) {
+    public static Boolean parseOptionsFromVM(String[] options, boolean parseOptionsFile) {
         try (InitTimer t = timer("ParseOptions")) {
             JVMCIJarsOptionDescriptorsProvider odp = new JVMCIJarsOptionDescriptorsProvider();
 
@@ -146,12 +86,17 @@
                 File jvmciOptions = new File(jvmci, "options");
                 if (jvmciOptions.exists()) {
                     try (BufferedReader br = new BufferedReader(new FileReader(jvmciOptions))) {
-                        String option = null;
-                        while ((option = br.readLine()) != null) {
-                            option = option.trim();
-                            if (!option.isEmpty() && option.charAt(0) != '#') {
-                                parseOption(option, null, odp);
+                        String optionSetting = null;
+                        int lineNo = 1;
+                        while ((optionSetting = br.readLine()) != null) {
+                            if (!optionSetting.isEmpty() && optionSetting.charAt(0) != '#') {
+                                try {
+                                    parseOptionSetting(optionSetting, null, odp);
+                                } catch (Throwable e) {
+                                    throw new InternalError("Error parsing " + jvmciOptions + ", line " + lineNo, e);
+                                }
                             }
+                            lineNo++;
                         }
                     } catch (IOException e) {
                         throw new InternalError("Error reading " + jvmciOptions, e);
@@ -160,106 +105,82 @@
             }
 
             if (options != null) {
-                int index = skip(options, 0, true);
-                while (index < options.length()) {
-                    int end = skip(options, index, false);
-                    String option = unescape(options.substring(index, end));
-                    parseOption(option, null, odp);
-                    index = skip(options, end, true);
+                assert options.length % 2 == 0;
+                for (int i = 0; i < options.length / 2; i++) {
+                    String name = options[i * 2];
+                    String value = options[i * 2 + 1];
+                    parseOption(OptionsLoader.options, name, value, null, odp);
                 }
             }
         }
         return Boolean.TRUE;
     }
 
-    public static void parseOption(String option, OptionConsumer setter, OptionDescriptorsProvider odp) {
-        parseOption(OptionsLoader.options, option, setter, odp);
+    /**
+     * Parses a given option setting.
+     *
+     * @param optionSetting a string matching the pattern {@code <name>=<value>}
+     * @param setter the object to notify of the parsed option and value
+     */
+    public static void parseOptionSetting(String optionSetting, OptionConsumer setter, OptionDescriptorsProvider odp) {
+        int eqIndex = optionSetting.indexOf('=');
+        if (eqIndex == -1) {
+            throw new InternalError("Option setting has does not match the pattern <name>=<value>: " + optionSetting);
+        }
+        String name = optionSetting.substring(0, eqIndex);
+        String value = optionSetting.substring(eqIndex + 1);
+        parseOption(OptionsLoader.options, name, value, setter, odp);
     }
 
     /**
-     * Parses a given option value specification.
+     * Parses a given option name and value.
      *
-     * @param option the specification of an option and its value
+     * @param name the option name
+     * @param valueString the option value as a string
      * @param setter the object to notify of the parsed option and value
      * @throws IllegalArgumentException if there's a problem parsing {@code option}
      */
-    public static void parseOption(SortedMap<String, OptionDescriptor> options, String option, OptionConsumer setter, OptionDescriptorsProvider odp) {
-        if (option.length() == 0) {
-            return;
-        }
-
-        Object value = null;
-        String optionName = null;
-        String valueString = null;
+    public static void parseOption(SortedMap<String, OptionDescriptor> options, String name, String valueString, OptionConsumer setter, OptionDescriptorsProvider odp) {
 
-        char first = option.charAt(0);
-        if (first == '+' || first == '-') {
-            optionName = option.substring(1);
-            value = (first == '+');
-        } else {
-            int index = option.indexOf('=');
-            if (index == -1) {
-                optionName = option;
-                valueString = null;
-            } else {
-                optionName = option.substring(0, index);
-                valueString = option.substring(index + 1);
-            }
-        }
-
-        OptionDescriptor desc = odp == null ? options.get(optionName) : odp.get(optionName);
-        if (desc == null && value != null) {
-            int index = option.indexOf('=');
-            if (index != -1) {
-                optionName = option.substring(1, index);
-                desc = odp == null ? options.get(optionName) : odp.get(optionName);
-            }
-            if (desc == null && optionName.equals("PrintFlags")) {
-                desc = OptionDescriptor.create("PrintFlags", Boolean.class, "Prints all JVMCI flags and exits", OptionsParser.class, "PrintFlags", PrintFlags);
-            }
+        OptionDescriptor desc = odp == null ? options.get(name) : odp.get(name);
+        if (desc == null && name.equals("PrintFlags")) {
+            desc = OptionDescriptor.create("PrintFlags", Boolean.class, "Prints all JVMCI flags and exits", OptionsParser.class, "PrintFlags", PrintFlags);
         }
         if (desc == null) {
-            List<OptionDescriptor> matches = fuzzyMatch(options, optionName);
+            List<OptionDescriptor> matches = fuzzyMatch(options, name);
             Formatter msg = new Formatter();
-            msg.format("Could not find option %s", optionName);
+            msg.format("Could not find option %s", name);
             if (!matches.isEmpty()) {
                 msg.format("%nDid you mean one of the following?");
                 for (OptionDescriptor match : matches) {
-                    boolean isBoolean = match.getType() == Boolean.class;
-                    msg.format("%n    %s%s%s", isBoolean ? "(+/-)" : "", match.getName(), isBoolean ? "" : "=<value>");
+                    msg.format("%n    %s=<value>", match.getName());
                 }
             }
             throw new IllegalArgumentException(msg.toString());
         }
 
         Class<?> optionType = desc.getType();
-
-        if (value == null) {
-            if (optionType == Boolean.TYPE || optionType == Boolean.class) {
-                throw new IllegalArgumentException("Boolean option '" + optionName + "' must use +/- prefix");
-            }
-
-            if (valueString == null) {
-                throw new IllegalArgumentException("Missing value for non-boolean option '" + optionName + "' must use " + optionName + "=<value> format");
+        Object value;
+        if (optionType == Boolean.class) {
+            if ("true".equals(valueString)) {
+                value = Boolean.TRUE;
+            } else if ("false".equals(valueString)) {
+                value = Boolean.FALSE;
+            } else {
+                throw new IllegalArgumentException("Boolean option '" + name + "' must have value \"true\" or \"false\", not \"" + valueString + "\"");
             }
-
-            if (optionType == Float.class) {
-                value = Float.parseFloat(valueString);
-            } else if (optionType == Double.class) {
-                value = Double.parseDouble(valueString);
-            } else if (optionType == Integer.class) {
-                value = Integer.valueOf((int) parseLong(valueString));
-            } else if (optionType == Long.class) {
-                value = Long.valueOf(parseLong(valueString));
-            } else if (optionType == String.class) {
-                value = valueString;
-            } else {
-                throw new IllegalArgumentException("Wrong value for option '" + optionName + "'");
-            }
+        } else if (optionType == Float.class) {
+            value = Float.parseFloat(valueString);
+        } else if (optionType == Double.class) {
+            value = Double.parseDouble(valueString);
+        } else if (optionType == Integer.class) {
+            value = Integer.valueOf((int) parseLong(valueString));
+        } else if (optionType == Long.class) {
+            value = Long.valueOf(parseLong(valueString));
+        } else if (optionType == String.class) {
+            value = valueString;
         } else {
-            if (optionType != Boolean.class) {
-                throw new IllegalArgumentException("Non-boolean option '" + optionName + "' can not use +/- prefix. Use " + optionName + "=<value> format");
-            }
+            throw new IllegalArgumentException("Wrong value for option '" + name + "'");
         }
         if (setter == null) {
             desc.getOptionValue().setValue(value);
--- a/mx.jvmci/mx_jvmci.py	Tue Sep 22 11:57:49 2015 -0700
+++ b/mx.jvmci/mx_jvmci.py	Tue Sep 22 22:29:28 2015 +0200
@@ -1758,22 +1758,15 @@
             args = jacocoArgs + args
 
         # Support for -G: options
-        jvmciArgs = []
-        nonJvmciArgs = []
-        existingJvmciOptionsProperty = None
-        for a in args:
-            if a.startswith('-G:'):
-                # Escape space with % and % with %%
-                jvmciArg = a[len('-G:'):].replace('%', '%%').replace(' ', '% ')
-                jvmciArgs.append(jvmciArg)
-            else:
-                if a.startswith('-Djvmci.options=') or a == '-Djvmci.options':
-                    existingJvmciOptionsProperty = a
-                nonJvmciArgs.append(a)
-        if jvmciArgs:
-            if existingJvmciOptionsProperty:
-                mx.abort('defining jvmci.option property is incompatible with defining one or more -G: options: ' + existingJvmciOptionsProperty)
-            args = ['-Djvmci.options=' + ' '.join(jvmciArgs)] + nonJvmciArgs
+        def translateGOption(arg):
+            if arg.startswith('-G:+'):
+                arg = '-Djvmci.option.' + arg[len('-G:+'):] + '=true'
+            elif arg.startswith('-G:-'):
+                arg = '-Djvmci.option.' + arg[len('-G:+'):] + '=false'
+            elif arg.startswith('-G:'):
+                arg = '-Djvmci.option.' + arg[len('-G:'):]
+            return arg
+        args = map(translateGOption, args)
 
         args = ['-Xbootclasspath/p:' + dep.classpath_repr() for dep in _jvmci_bootclasspath_prepends] + args
 
--- a/src/share/vm/jvmci/jvmciRuntime.cpp	Tue Sep 22 11:57:49 2015 -0700
+++ b/src/share/vm/jvmci/jvmciRuntime.cpp	Tue Sep 22 22:29:28 2015 +0200
@@ -48,11 +48,15 @@
 jobject JVMCIRuntime::_HotSpotJVMCIRuntime_instance = NULL;
 bool JVMCIRuntime::_HotSpotJVMCIRuntime_initialized = false;
 const char* JVMCIRuntime::_compiler = NULL;
-const char* JVMCIRuntime::_options = NULL;
+int JVMCIRuntime::_options_count = 0;
+SystemProperty** JVMCIRuntime::_options = NULL;
 int JVMCIRuntime::_trivial_prefixes_count = 0;
 char** JVMCIRuntime::_trivial_prefixes = NULL;
 bool JVMCIRuntime::_shutdown_called = false;
 
+static const char* OPTION_PREFIX = "jvmci.option.";
+static const int OPTION_PREFIX_LEN = strlen(OPTION_PREFIX);
+
 void JVMCIRuntime::initialize_natives(JNIEnv *env, jclass c2vmClass) {
 #ifdef _LP64
 #ifndef TARGET_ARCH_sparc
@@ -688,12 +692,24 @@
     bool parseOptionsFile = jvmci_options_file_exists();
     if (_options != NULL || parseOptionsFile) {
       JavaCallArguments args;
-      oop options = java_lang_String::create_oop_from_str(_options, CHECK);
+      objArrayOop options;
+      if (_options != NULL) {
+        options = oopFactory::new_objArray(SystemDictionary::String_klass(), _options_count * 2, CHECK);
+        for (int i = 0; i < _options_count; i++) {
+          SystemProperty* prop = _options[i];
+          oop name = java_lang_String::create_oop_from_str(prop->key() + OPTION_PREFIX_LEN, CHECK);
+          oop value = java_lang_String::create_oop_from_str(prop->value(), CHECK);
+          options->obj_at_put(i * 2, name);
+          options->obj_at_put((i * 2) + 1, value);
+        }
+      } else {
+        options = NULL;
+      }
       args.push_oop(options);
       args.push_int(parseOptionsFile);
       callStatic("jdk/internal/jvmci/options/OptionsParser",
                  "parseOptionsFromVM",
-                 "(Ljava/lang/String;Z)Ljava/lang/Boolean;", &args, CHECK);
+                 "([Ljava/lang/String;Z)Ljava/lang/Boolean;", &args, CHECK);
     }
 
     if (_compiler != NULL) {
@@ -927,10 +943,35 @@
   _compiler = compiler;
 }
 
-void JVMCIRuntime::save_options(const char* options) {
-  assert(options != NULL, "npe");
-  assert(_options == NULL, "cannot reassign JVMCI options");
-  _options = options;
+jint JVMCIRuntime::save_options(SystemProperty* props) {
+  int count = 0;
+  SystemProperty* first = NULL;
+  for (SystemProperty* p = props; p != NULL; p = p->next()) {
+    if (strncmp(p->key(), OPTION_PREFIX, OPTION_PREFIX_LEN) == 0) {
+      if (p->value() == NULL || strlen(p->value()) == 0) {
+        jio_fprintf(defaultStream::output_stream(), "JVMCI option %s must have non-zero length value\n", p->key());
+        return JNI_ERR;
+      }
+      if (first == NULL) {
+        first = p;
+      }
+      count++;
+    }
+  }
+  if (count != 0) {
+    _options_count = count;
+    _options = NEW_C_HEAP_ARRAY(SystemProperty*, count, mtCompiler);
+    _options[0] = first;
+    SystemProperty** insert_pos = _options + 1;
+    for (SystemProperty* p = first->next(); p != NULL; p = p->next()) {
+      if (strncmp(p->key(), OPTION_PREFIX, OPTION_PREFIX_LEN) == 0) {
+        *insert_pos = p;
+        insert_pos++;
+      }
+    }
+    assert (insert_pos - _options == count, "must be");
+  }
+  return JNI_OK;
 }
 
 Handle JVMCIRuntime::create_Service(const char* name, TRAPS) {
--- a/src/share/vm/jvmci/jvmciRuntime.hpp	Tue Sep 22 11:57:49 2015 -0700
+++ b/src/share/vm/jvmci/jvmciRuntime.hpp	Tue Sep 22 22:29:28 2015 +0200
@@ -59,7 +59,8 @@
   static jobject _HotSpotJVMCIRuntime_instance;
   static bool _HotSpotJVMCIRuntime_initialized;
   static const char* _compiler;
-  static const char* _options;
+  static int _options_count;
+  static SystemProperty** _options;
 
   static int _trivial_prefixes_count;
   static char** _trivial_prefixes;
@@ -87,10 +88,13 @@
   static void save_compiler(const char* compiler);
 
   /**
-   * Saves the value of the "jvmci.options" system property for processing
+   * Saves the value of the system properties starting with "jvmci.option." for processing
    * when JVMCI is initialized.
+   *
+   * @param props the head of the system property list
+   * @return JNI_ERR if a JVMCI option has a zero length value, JNI_OK otherwise
    */
-  static void save_options(const char* options);
+  static jint save_options(SystemProperty* props);
 
   /**
    * Ensures that the JVMCI class loader is initialized and the well known JVMCI classes are loaded.
--- a/src/share/vm/runtime/arguments.cpp	Tue Sep 22 11:57:49 2015 -0700
+++ b/src/share/vm/runtime/arguments.cpp	Tue Sep 22 22:29:28 2015 +0200
@@ -3570,6 +3570,11 @@
   scp_p->expand_endorsed();
 
 #if INCLUDE_JVMCI
+  jint res = JVMCIRuntime::save_options(_system_properties);
+  if (res != JNI_OK) {
+    return res;
+  }
+
   if (!UseJVMCIClassLoader) {
     // Append lib/jvmci/*.jar to boot class path
     char jvmciDir[JVM_MAXPATHLEN];
--- a/src/share/vm/runtime/thread.cpp	Tue Sep 22 11:57:49 2015 -0700
+++ b/src/share/vm/runtime/thread.cpp	Tue Sep 22 22:29:28 2015 +0200
@@ -3706,10 +3706,6 @@
   if (jvmciCompiler != NULL) {
     JVMCIRuntime::save_compiler(jvmciCompiler);
   }
-  const char* jvmciOptions = Arguments::PropertyList_get_value(Arguments::system_properties(), "jvmci.options");
-  if (jvmciOptions != NULL) {
-    JVMCIRuntime::save_options(jvmciOptions);
-  }
 #endif
 
   // initialize compiler(s)