From 75d6c996f1ef91849e42cd98d775684a7b190d99 Mon Sep 17 00:00:00 2001 From: Attila Szegedi Date: Mon, 9 Nov 2015 15:37:07 +0100 Subject: [PATCH] 8141550: Introduce a command line option instead of nashorn.unstable.relink.threshold system property Reviewed-by: hannesw, sundar --- nashorn/docs/DEVELOPER_README | 18 ++++++++---- .../jdk/nashorn/internal/runtime/Context.java | 2 +- .../internal/runtime/ScriptEnvironment.java | 25 +++++++++++++++++ .../internal/runtime/linker/Bootstrap.java | 28 +++---------------- .../runtime/resources/Options.properties | 12 ++++++++ nashorn/test/script/basic/JDK-8011578.js | 3 +- nashorn/test/script/basic/JDK-8044750.js | 3 +- nashorn/test/script/basic/JDK-8136544.js | 3 +- nashorn/test/script/basic/JDK-8136694.js | 3 +- 9 files changed, 58 insertions(+), 39 deletions(-) diff --git a/nashorn/docs/DEVELOPER_README b/nashorn/docs/DEVELOPER_README index faff28c0bac..deb525d5ff4 100644 --- a/nashorn/docs/DEVELOPER_README +++ b/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 -callsite is relinked with "apply" semantics to never change again. -In the case of megamorphic callsites, this is necessary, or the -program would spend all its time swapping out callsite targets. Dynalink -has a default value (currently 8 relinks) for this property if it -is not explicitly set. +NOTE: This property is deprecated in favor of the +"--unstable-relink-threshold" command line option. It controls how many +call site misses are allowed before a callsite is relinked with "apply" +semantics to never change again. In the case of megamorphic callsites, +this is necessary, or the program would spend all its time swapping out +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 diff --git a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/Context.java b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/Context.java index ca9e2719d8b..790c5225434 100644 --- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/Context.java +++ b/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) { diff --git a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/ScriptEnvironment.java b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/ScriptEnvironment.java index d09f8f04559..8f8774f8451 100644 --- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/ScriptEnvironment.java +++ b/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; diff --git a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/linker/Bootstrap.java b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/linker/Bootstrap.java index 3c0db231b9d..ea172a74b08 100644 --- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/linker/Bootstrap.java +++ b/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(UNSTABLE_RELINK_THRESHOLD); - } + factory.setUnstableRelinkThreshold(unstableRelinkThreshold); // Linkers for any additional language runtimes deployed alongside Nashorn will be picked up by the factory. factory.setClassLoader(appLoader); diff --git a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/resources/Options.properties b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/resources/Options.properties index c487c86b652..b7a9e460535 100644 --- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/resources/Options.properties +++ b/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, \ diff --git a/nashorn/test/script/basic/JDK-8011578.js b/nashorn/test/script/basic/JDK-8011578.js index 162c61b302e..87107bd6dd7 100644 --- a/nashorn/test/script/basic/JDK-8011578.js +++ b/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 - * @fork + * @option --unstable-relink-threshold=1 * @run */ diff --git a/nashorn/test/script/basic/JDK-8044750.js b/nashorn/test/script/basic/JDK-8044750.js index 0123fb86761..d1baa73635a 100644 --- a/nashorn/test/script/basic/JDK-8044750.js +++ b/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 -Dnashorn.unstable.relink.threshold=16 + * @option --unstable-relink-threshold=16 * @run */ diff --git a/nashorn/test/script/basic/JDK-8136544.js b/nashorn/test/script/basic/JDK-8136544.js index b1eeb358f7d..4a4385bcdbc 100644 --- a/nashorn/test/script/basic/JDK-8136544.js +++ b/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 -Dnashorn.unstable.relink.threshold=8 + * @option --unstable-relink-threshold=8 * @run */ diff --git a/nashorn/test/script/basic/JDK-8136694.js b/nashorn/test/script/basic/JDK-8136694.js index 29009e4fc2e..974ab37cdd5 100644 --- a/nashorn/test/script/basic/JDK-8136694.js +++ b/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 -Dnashorn.unstable.relink.threshold=16 + * @option --unstable-relink-threshold=16 * @run */