8141550: Introduce a command line option instead of nashorn.unstable.relink.threshold system property

Reviewed-by: hannesw, sundar

nashorn/docs/DEVELOPER_README

@@ -35,12 +35,14 @@ that can be overwritten.
 
 SYSTEM PROPERTY: -Dnashorn.unstable.relink.threshold=x
 
-This property controls how many call site misses are allowed before a 
+NOTE: This property is deprecated in favor of the 
-callsite is relinked with "apply" semantics to never change again. 
+"--unstable-relink-threshold" command line option. It controls how many
-In the case of megamorphic callsites, this is necessary, or the 
+call site misses are allowed before a callsite is relinked with "apply"
-program would spend all its time swapping out callsite targets. Dynalink 
+semantics to never change again. In the case of megamorphic callsites, 
-has a default value (currently 8 relinks) for this property if it 
+this is necessary, or the program would spend all its time swapping out 
-is not explicitly set.
+callsite targets. When neither the system property nor the command line
+option are specified, defaults to 8, or 16 with optimistic types turned
+on.
 
 
 SYSTEM PROPERTY: -Dnashorn.compiler.splitter.threshold=x
@@ -607,6 +609,10 @@ A short summary follows:
 	                         enterexit [trace callsite enter/exit], objects [print object properties].)
 		param: [=[option,]*]   
 
+	-urt, --unstable-relink-threshold (Number of times a dynamic call site has to be relinked before it 
+	                                  is considered unstable, when the runtime will try to link it as 
+	                                  if it is megamorphic.)
+
 	--verify-code (Verify byte code before running.)
 		param: [true|false]   default: false
 

nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/Context.java

@@ -649,7 +649,7 @@ public final class Context {
         } else {
             this.appLoader = appLoader;
         }
-        this.dynamicLinker = Bootstrap.createDynamicLinker(this.appLoader);
+        this.dynamicLinker = Bootstrap.createDynamicLinker(this.appLoader, env._unstable_relink_threshold);
 
         final int cacheSize = env._class_cache_size;
         if (cacheSize > 0) {

nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/ScriptEnvironment.java

@@ -105,6 +105,12 @@ public final class ScriptEnvironment {
     /** Enable experimental ECMAScript 6 features. */
     public final boolean _es6;
 
+
+    /** Number of times a dynamic call site has to be relinked before it is
+     * considered unstable (and thus should be linked as if it were megamorphic).
+     */
+    public final int _unstable_relink_threshold;
+
     /** Argument passed to compile only if optimistic compilation should take place */
     public static final String COMPILE_ONLY_OPTIMISTIC_ARG = "optimistic";
 
@@ -287,6 +293,25 @@ public final class ScriptEnvironment {
         _version              = options.getBoolean("version");
         _verify_code          = options.getBoolean("verify.code");
 
+        final int configuredUrt = options.getInteger("unstable.relink.threshold");
+        // The default for this property is -1, so we can easily detect when
+        // it is not specified on command line.
+        if (configuredUrt < 0) {
+            // In this case, use a default of 8, or 16 for optimistic types.
+            // Optimistic types come with dual fields, and in order to get
+            // performance on benchmarks with a lot of object instantiation and
+            // then field reassignment, it can take slightly more relinks to
+            // become stable with type changes swapping out an entire property
+            // map and making a map guard fail. Also, honor the "nashorn.*"
+            // system property for now. It was documented in DEVELOPER_README
+            // so we should recognize it for the time being.
+            _unstable_relink_threshold = Options.getIntProperty(
+                    "nashorn.unstable.relink.threshold",
+                    _optimistic_types ? 16 : 8);
+        } else {
+            _unstable_relink_threshold = configuredUrt;
+        }
+
         final String anonClasses = options.getString("anonymous.classes");
         if (anonClasses == null || anonClasses.equals("auto")) {
             _anonymousClasses = AnonymousClasses.AUTO;

nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/linker/Bootstrap.java

@@ -55,7 +55,6 @@ import jdk.nashorn.internal.runtime.JSType;
 import jdk.nashorn.internal.runtime.OptimisticReturnFilters;
 import jdk.nashorn.internal.runtime.ScriptFunction;
 import jdk.nashorn.internal.runtime.ScriptRuntime;
-import jdk.nashorn.internal.runtime.options.Options;
 
 /**
  * This class houses bootstrap method for invokedynamic instructions generated by compiler.
@@ -68,25 +67,6 @@ public final class Bootstrap {
 
     private static final MethodHandle VOID_TO_OBJECT = MH.constant(Object.class, ScriptRuntime.UNDEFINED);
 
-    /**
-     * The default dynalink relink threshold for megamorphism is 8. In the case
-     * of object fields only, it is fine. However, with dual fields, in order to get
-     * performance on benchmarks with a lot of object instantiation and then field
-     * reassignment, it can take slightly more relinks to become stable with type
-     * changes swapping out an entire property map and making a map guard fail.
-     * Since we need to set this value statically it must work with possibly changing
-     * optimistic types and dual fields settings. A higher value does not seem to have
-     * any other negative performance implication when running with object-only fields,
-     * so we choose a higher value here.
-     *
-     * See for example octane.gbemu, run with --log=fields:warning to study
-     * megamorphic behavior
-     */
-    private static final int UNSTABLE_RELINK_THRESHOLD_DEFAULT = 16;
-    private static final int UNSTABLE_RELINK_THRESHOLD =
-            Options.getIntProperty("nashorn.unstable.relink.threshold",
-                    UNSTABLE_RELINK_THRESHOLD_DEFAULT);
-
     // do not create me!!
     private Bootstrap() {
     }
@@ -95,9 +75,11 @@ public final class Bootstrap {
      * Creates a Nashorn dynamic linker with the given app class loader.
      * @param appLoader the app class loader. It will be used to discover
      * additional language runtime linkers (if any).
+     * @param unstableRelinkThreshold the unstable relink threshold
      * @return a newly created dynamic linker.
      */
-    public static DynamicLinker createDynamicLinker(final ClassLoader appLoader) {
+    public static DynamicLinker createDynamicLinker(final ClassLoader appLoader,
+            final int unstableRelinkThreshold) {
         final DynamicLinkerFactory factory = new DynamicLinkerFactory();
         final NashornBeansLinker nashornBeansLinker = new NashornBeansLinker();
         factory.setPrioritizedLinkers(
@@ -125,9 +107,7 @@ public final class Bootstrap {
             }
         });
         factory.setInternalObjectsFilter(NashornBeansLinker.createHiddenObjectFilter());
-        if (UNSTABLE_RELINK_THRESHOLD > -1) {
+        factory.setUnstableRelinkThreshold(unstableRelinkThreshold);
-            factory.setUnstableRelinkThreshold(UNSTABLE_RELINK_THRESHOLD);
-        }
 
         // Linkers for any additional language runtimes deployed alongside Nashorn will be picked up by the factory.
         factory.setClassLoader(appLoader);

nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/resources/Options.properties

@@ -389,6 +389,18 @@ nashorn.option.anonymous.classes = {                      \
     desc="Use VM anonymous classes for compiled scripts." \
 }
 
+nashorn.option.unstable.relink.threshold ={                   \
+    name="--unstable-relink-threshold",                       \
+    short_name="-urt",                                        \
+    desc="Number of times a dynamic call site has to be \
+    relinked before it is considered unstable, when the \
+    runtime will try to link it as if it is megamorphic.",    \
+    is_undocumented=true,                                     \
+    type=Integer,                                             \
+    default=-1                                                \
+}
+
+
 nashorn.option.verify.code = {              \
     name="--verify-code",                   \
     is_undocumented=true,                   \

nashorn/test/script/basic/JDK-8011578.js

@@ -25,8 +25,7 @@
  * JDK-8011578 : -Dnashorn.unstable.relink.threshold=1 causes tests to fail.
  *
  * @test
- * @option -Dnashorn.unstable.relink.threshold=1
+ * @option --unstable-relink-threshold=1
- * @fork
  * @run
  */
 

nashorn/test/script/basic/JDK-8044750.js

@@ -25,8 +25,7 @@
  * JDK-8044750: megamorphic getter for scope objects does not call __noSuchProperty__ hook
  *
  * @test
- * @fork
+ * @option --unstable-relink-threshold=16
- * @option -Dnashorn.unstable.relink.threshold=16
  * @run
  */
 

nashorn/test/script/basic/JDK-8136544.js

@@ -25,8 +25,7 @@
  * JDK-8136544: Call site switching to megamorphic causes incorrect property read
  *
  * @test
- * @fork
+ * @option --unstable-relink-threshold=8
- * @option -Dnashorn.unstable.relink.threshold=8
  * @run
  */
 

nashorn/test/script/basic/JDK-8136694.js

@@ -25,8 +25,7 @@
  * JDK-8136694: Megemorphic scope access does not throw ReferenceError when property is missing
  *
  * @test
- * @fork
+ * @option --unstable-relink-threshold=16
- * @option -Dnashorn.unstable.relink.threshold=16
  * @run
  */