# HG changeset patch # User hannesw # Date 1430131821 -7200 # Node ID 248dc4f11e5bbf4012994b4dc1f5cfb147a5f2fc # Parent 5ed57fe26f13900f7cc5cbb066fab8e564cc9dec 8053905: Eager code generation fails for earley boyer with split threshold set to 1000 Reviewed-by: attila, lagergren diff -r 5ed57fe26f13 -r 248dc4f11e5b src/jdk/nashorn/internal/codegen/CodeGenerator.java --- a/src/jdk/nashorn/internal/codegen/CodeGenerator.java Mon Apr 27 12:27:33 2015 +0200 +++ b/src/jdk/nashorn/internal/codegen/CodeGenerator.java Mon Apr 27 12:50:21 2015 +0200 @@ -4526,7 +4526,7 @@ } if (addInitializer && !compiler.isOnDemandCompilation()) { - compiler.addFunctionInitializer(data, functionNode); + functionNode.getCompileUnit().addFunctionInitializer(data, functionNode); } // We don't emit a ScriptFunction on stack for the outermost compiled function (as there's no code being diff -r 5ed57fe26f13 -r 248dc4f11e5b src/jdk/nashorn/internal/codegen/CompilationPhase.java --- a/src/jdk/nashorn/internal/codegen/CompilationPhase.java Mon Apr 27 12:27:33 2015 +0200 +++ b/src/jdk/nashorn/internal/codegen/CompilationPhase.java Mon Apr 27 12:50:21 2015 +0200 @@ -57,7 +57,6 @@ import jdk.nashorn.internal.ir.debug.PrintVisitor; import jdk.nashorn.internal.ir.visitor.NodeVisitor; import jdk.nashorn.internal.runtime.CodeInstaller; -import jdk.nashorn.internal.runtime.FunctionInitializer; import jdk.nashorn.internal.runtime.RecompilableScriptFunctionData; import jdk.nashorn.internal.runtime.ScriptEnvironment; import jdk.nashorn.internal.runtime.logging.DebugLogger; @@ -581,18 +580,7 @@ continue; } unit.setCode(installedClasses.get(unit.getUnitClassName())); - } - - if (!compiler.isOnDemandCompilation()) { - // Initialize functions - final Map initializers = compiler.getFunctionInitializers(); - if (initializers != null) { - for (final Entry entry : initializers.entrySet()) { - final FunctionInitializer initializer = entry.getValue(); - initializer.setCode(installedClasses.get(initializer.getClassName())); - compiler.getScriptFunctionData(entry.getKey()).initializeCode(initializer); - } - } + unit.initializeFunctionsCode(); } if (log.isEnabled()) { diff -r 5ed57fe26f13 -r 248dc4f11e5b src/jdk/nashorn/internal/codegen/CompileUnit.java --- a/src/jdk/nashorn/internal/codegen/CompileUnit.java Mon Apr 27 12:27:33 2015 +0200 +++ b/src/jdk/nashorn/internal/codegen/CompileUnit.java Mon Apr 27 12:50:21 2015 +0200 @@ -26,10 +26,16 @@ package jdk.nashorn.internal.codegen; import java.io.Serializable; +import java.util.Collection; +import java.util.Collections; +import java.util.IdentityHashMap; +import java.util.Map; import java.util.Objects; import java.util.Set; import java.util.TreeSet; import jdk.nashorn.internal.ir.CompileUnitHolder; +import jdk.nashorn.internal.ir.FunctionNode; +import jdk.nashorn.internal.runtime.RecompilableScriptFunctionData; /** * Used to track split class compilation. Note that instances of the class are serializable, but all fields are @@ -50,6 +56,8 @@ private transient Class clazz; + private transient Map functions = new IdentityHashMap<>(); + private transient boolean isUsed; private static int emittedUnitCount; @@ -121,6 +129,32 @@ this.classEmitter = null; } + void addFunctionInitializer(final RecompilableScriptFunctionData data, final FunctionNode functionNode) { + functions.put(functionNode, data); + } + + /** + * Returns true if this compile unit is responsible for initializing the specified function data with specified + * function node. + * @param data the function data to check + * @param functionNode the function node to check + * @return true if this unit is responsible for initializing the function data with the function node, otherwise + * false + */ + public boolean isInitializing(final RecompilableScriptFunctionData data, final FunctionNode functionNode) { + return functions.get(functionNode) == data; + } + + void initializeFunctionsCode() { + for(final Map.Entry entry : functions.entrySet()) { + entry.getValue().initializeCode(entry.getKey()); + } + } + + Collection getFunctionNodes() { + return Collections.unmodifiableCollection(functions.keySet()); + } + /** * Add weight to this compile unit * @param w weight to add diff -r 5ed57fe26f13 -r 248dc4f11e5b src/jdk/nashorn/internal/codegen/Compiler.java --- a/src/jdk/nashorn/internal/codegen/Compiler.java Mon Apr 27 12:27:33 2015 +0200 +++ b/src/jdk/nashorn/internal/codegen/Compiler.java Mon Apr 27 12:50:21 2015 +0200 @@ -565,7 +565,7 @@ * @return a copy of this compiler's current mapping of invalidated optimistic program points to their types. */ public Map getInvalidatedProgramPoints() { - return invalidatedProgramPoints == null ? null : new TreeMap<>(invalidatedProgramPoints); + return invalidatedProgramPoints.isEmpty() ? null : new TreeMap<>(invalidatedProgramPoints); } TypeMap getTypeMap() { @@ -704,21 +704,6 @@ return sb.toString(); } - Map functionInitializers; - - void addFunctionInitializer(final RecompilableScriptFunctionData functionData, final FunctionNode functionNode) { - if (functionInitializers == null) { - functionInitializers = new HashMap<>(); - } - if (!functionInitializers.containsKey(functionData)) { - functionInitializers.put(functionData.getFunctionNodeId(), new FunctionInitializer(functionNode)); - } - } - - Map getFunctionInitializers() { - return functionInitializers; - } - /** * Persist current compilation with the given {@code cacheKey}. * @param cacheKey cache key @@ -726,15 +711,17 @@ */ public void persistClassInfo(final String cacheKey, final FunctionNode functionNode) { if (cacheKey != null && env._persistent_cache) { - Map initializers; // If this is an on-demand compilation create a function initializer for the function being compiled. // Otherwise use function initializer map generated by codegen. - if (functionInitializers == null) { - initializers = new HashMap<>(); - final FunctionInitializer initializer = new FunctionInitializer(functionNode, getInvalidatedProgramPoints()); - initializers.put(functionNode.getId(), initializer); + Map initializers = new HashMap<>(); + if (isOnDemandCompilation()) { + initializers.put(functionNode.getId(), new FunctionInitializer(functionNode, getInvalidatedProgramPoints())); } else { - initializers = functionInitializers; + for (final CompileUnit compileUnit : getCompileUnits()) { + for (final FunctionNode fn : compileUnit.getFunctionNodes()) { + initializers.put(fn.getId(), new FunctionInitializer(fn)); + } + } } final String mainClassName = getFirstCompileUnit().getUnitClassName(); installer.storeScript(cacheKey, source, mainClassName, bytecode, initializers, constantData.toArray(), compilationId); diff -r 5ed57fe26f13 -r 248dc4f11e5b src/jdk/nashorn/internal/codegen/TypeEvaluator.java --- a/src/jdk/nashorn/internal/codegen/TypeEvaluator.java Mon Apr 27 12:27:33 2015 +0200 +++ b/src/jdk/nashorn/internal/codegen/TypeEvaluator.java Mon Apr 27 12:50:21 2015 +0200 @@ -227,7 +227,8 @@ // gradually introduce them as needed. An easy one would be to do the same for .call(this) idiom. final CallNode callExpr = (CallNode)expr; final Expression fnExpr = callExpr.getFunction(); - if (fnExpr instanceof FunctionNode) { + // Skip evaluation if running with eager compilation as we may violate constraints in RecompilableScriptFunctionData + if (fnExpr instanceof FunctionNode && compiler.getContext().getEnv()._lazy_compilation) { final FunctionNode fn = (FunctionNode)fnExpr; if (callExpr.getArgs().isEmpty()) { final RecompilableScriptFunctionData data = compiler.getScriptFunctionData(fn.getId()); diff -r 5ed57fe26f13 -r 248dc4f11e5b src/jdk/nashorn/internal/runtime/Context.java --- a/src/jdk/nashorn/internal/runtime/Context.java Mon Apr 27 12:27:33 2015 +0200 +++ b/src/jdk/nashorn/internal/runtime/Context.java Mon Apr 27 12:50:21 2015 +0200 @@ -1281,7 +1281,7 @@ compiler.persistClassInfo(cacheKey, compiledFunction); } else { Compiler.updateCompilationId(storedScript.getCompilationId()); - script = install(storedScript, source, installer); + script = storedScript.installScript(source, installer); } cacheClass(source, script); @@ -1303,51 +1303,6 @@ } /** - * Install a previously compiled class from the code cache. - * - * @param storedScript cached script containing class bytes and constants - * @return main script class - */ - private static Class install(final StoredScript storedScript, final Source source, final CodeInstaller installer) { - - final Map> installedClasses = new HashMap<>(); - final Map classBytes = storedScript.getClassBytes(); - final Object[] constants = storedScript.getConstants(); - final String mainClassName = storedScript.getMainClassName(); - final byte[] mainClassBytes = classBytes.get(mainClassName); - final Class mainClass = installer.install(mainClassName, mainClassBytes); - final Map initializers = storedScript.getInitializers(); - - installedClasses.put(mainClassName, mainClass); - - for (final Map.Entry entry : classBytes.entrySet()) { - final String className = entry.getKey(); - if (className.equals(mainClassName)) { - continue; - } - final byte[] code = entry.getValue(); - - installedClasses.put(className, installer.install(className, code)); - } - - installer.initialize(installedClasses.values(), source, constants); - - for (final Object constant : constants) { - if (constant instanceof RecompilableScriptFunctionData) { - final RecompilableScriptFunctionData data = (RecompilableScriptFunctionData) constant; - data.initTransients(source, installer); - final FunctionInitializer initializer = initializers.get(data.getFunctionNodeId()); - if (initializer != null) { - initializer.setCode(installedClasses.get(initializer.getClassName())); - data.initializeCode(initializer); - } - } - } - - return mainClass; - } - - /** * Cache for compiled script classes. */ @SuppressWarnings("serial") diff -r 5ed57fe26f13 -r 248dc4f11e5b src/jdk/nashorn/internal/runtime/FunctionInitializer.java --- a/src/jdk/nashorn/internal/runtime/FunctionInitializer.java Mon Apr 27 12:27:33 2015 +0200 +++ b/src/jdk/nashorn/internal/runtime/FunctionInitializer.java Mon Apr 27 12:50:21 2015 +0200 @@ -60,17 +60,6 @@ } /** - * Copy constructor. - * - * @param init original initializer - */ - FunctionInitializer(final FunctionInitializer init) { - this.className = init.getClassName(); - this.methodType = init.getMethodType(); - this.flags = init.getFlags(); - } - - /** * Constructor. * * @param functionNode the function node @@ -130,7 +119,7 @@ * Set the class implementing the function * @param code the class */ - public void setCode(final Class code) { + void setCode(final Class code) { // Make sure code has not been set and has expected class name if (this.code != null) { throw new IllegalStateException("code already set"); diff -r 5ed57fe26f13 -r 248dc4f11e5b src/jdk/nashorn/internal/runtime/RecompilableScriptFunctionData.java --- a/src/jdk/nashorn/internal/runtime/RecompilableScriptFunctionData.java Mon Apr 27 12:27:33 2015 +0200 +++ b/src/jdk/nashorn/internal/runtime/RecompilableScriptFunctionData.java Mon Apr 27 12:50:21 2015 +0200 @@ -32,7 +32,6 @@ import java.lang.invoke.MethodType; import java.util.Collection; import java.util.Collections; -import java.util.HashMap; import java.util.HashSet; import java.util.Map; import java.util.Set; @@ -503,7 +502,7 @@ if (script != null) { Compiler.updateCompilationId(script.getCompilationId()); - return installStoredScript(script, newInstaller); + return script.installFunction(this, newInstaller); } } @@ -518,56 +517,6 @@ return new FunctionInitializer(compiledFn, compiler.getInvalidatedProgramPoints()); } - private static Map> installStoredScriptClasses(final StoredScript script, final CodeInstaller installer) { - final Map> installedClasses = new HashMap<>(); - final Map classBytes = script.getClassBytes(); - final String mainClassName = script.getMainClassName(); - final byte[] mainClassBytes = classBytes.get(mainClassName); - - final Class mainClass = installer.install(mainClassName, mainClassBytes); - - installedClasses.put(mainClassName, mainClass); - - for (final Map.Entry entry : classBytes.entrySet()) { - final String className = entry.getKey(); - final byte[] bytecode = entry.getValue(); - - if (className.equals(mainClassName)) { - continue; - } - - installedClasses.put(className, installer.install(className, bytecode)); - } - return installedClasses; - } - - /** - * Install this script using the given {@code installer}. - * - * @param script the compiled script - * @return the function initializer - */ - private FunctionInitializer installStoredScript(final StoredScript script, final CodeInstaller newInstaller) { - final Map> installedClasses = installStoredScriptClasses(script, newInstaller); - - final Map initializers = script.getInitializers(); - assert initializers != null; - assert initializers.size() == 1; - final FunctionInitializer initializer = initializers.values().iterator().next(); - - final Object[] constants = script.getConstants(); - for (int i = 0; i < constants.length; i++) { - if (constants[i] instanceof RecompilableScriptFunctionData) { - // replace deserialized function data with the ones we already have - constants[i] = getScriptFunctionData(((RecompilableScriptFunctionData) constants[i]).getFunctionNodeId()); - } - } - - newInstaller.initialize(installedClasses.values(), source, constants); - initializer.setCode(installedClasses.get(initializer.getClassName())); - return initializer; - } - boolean usePersistentCodeCache() { final ScriptEnvironment env = installer.getOwner(); return env._persistent_cache && env._optimistic_types; @@ -645,13 +594,21 @@ * by the compiler internals in Nashorn and is public for implementation reasons only. Attempting to invoke it * externally will result in an exception. * - * @param initializer FunctionInitializer for this data + * @param functionNode FunctionNode for this data */ - public void initializeCode(final FunctionInitializer initializer) { + public void initializeCode(final FunctionNode functionNode) { // Since the method is public, we double-check that we aren't invoked with an inappropriate compile unit. - if(!code.isEmpty()) { + if (!code.isEmpty() || functionNode.getId() != functionNodeId || !functionNode.getCompileUnit().isInitializing(this, functionNode)) { throw new IllegalStateException(name); } + addCode(lookup(functionNode), null, null, functionNode.getFlags()); + } + + /** + * Initializes this function with the given function code initializer. + * @param initializer function code initializer + */ + void initializeCode(final FunctionInitializer initializer) { addCode(lookup(initializer, true), null, null, initializer.getFlags()); } diff -r 5ed57fe26f13 -r 248dc4f11e5b src/jdk/nashorn/internal/runtime/StoredScript.java --- a/src/jdk/nashorn/internal/runtime/StoredScript.java Mon Apr 27 12:27:33 2015 +0200 +++ b/src/jdk/nashorn/internal/runtime/StoredScript.java Mon Apr 27 12:50:21 2015 +0200 @@ -27,7 +27,7 @@ import java.io.Serializable; import java.util.Arrays; -import java.util.LinkedHashMap; +import java.util.HashMap; import java.util.Map; /** @@ -77,44 +77,70 @@ return compilationId; } - /** - * Returns the main class name. - * @return the main class name - */ - public String getMainClassName() { - return mainClassName; + private Map> installClasses(final Source source, final CodeInstaller installer) { + final Map> installedClasses = new HashMap<>(); + final byte[] mainClassBytes = classBytes.get(mainClassName); + final Class mainClass = installer.install(mainClassName, mainClassBytes); + + installedClasses.put(mainClassName, mainClass); + + for (final Map.Entry entry : classBytes.entrySet()) { + final String className = entry.getKey(); + + if (!className.equals(mainClassName)) { + installedClasses.put(className, installer.install(className, entry.getValue())); + } + } + + installer.initialize(installedClasses.values(), source, constants); + return installedClasses; + } + + FunctionInitializer installFunction(final RecompilableScriptFunctionData data, final CodeInstaller installer) { + final Map> installedClasses = installClasses(data.getSource(), installer); + + assert initializers != null; + assert initializers.size() == 1; + final FunctionInitializer initializer = initializers.values().iterator().next(); + + for (int i = 0; i < constants.length; i++) { + if (constants[i] instanceof RecompilableScriptFunctionData) { + // replace deserialized function data with the ones we already have + final RecompilableScriptFunctionData newData = data.getScriptFunctionData(((RecompilableScriptFunctionData) constants[i]).getFunctionNodeId()); + assert newData != null; + newData.initTransients(data.getSource(), installer); + constants[i] = newData; + } + } + + initializer.setCode(installedClasses.get(initializer.getClassName())); + return initializer; } /** - * Returns a map of class names to class bytes. - * @return map of class bytes + * Install as script. + * + * @param source the source + * @param installer the installer + * @return main script class */ - public Map getClassBytes() { - final Map clonedMap = new LinkedHashMap<>(); - for (final Map.Entry entry : classBytes.entrySet()) { - clonedMap.put(entry.getKey(), entry.getValue().clone()); + Class installScript(final Source source, final CodeInstaller installer) { + + final Map> installedClasses = installClasses(source, installer); + + for (final Object constant : constants) { + if (constant instanceof RecompilableScriptFunctionData) { + final RecompilableScriptFunctionData data = (RecompilableScriptFunctionData) constant; + data.initTransients(source, installer); + final FunctionInitializer initializer = initializers.get(data.getFunctionNodeId()); + if (initializer != null) { + initializer.setCode(installedClasses.get(initializer.getClassName())); + data.initializeCode(initializer); + } + } } - return clonedMap; - } - /** - * Returns the constants array. - * @return constants array - */ - public Object[] getConstants() { - return constants.clone(); - } - - /** - * Returns the function initializers map. - * @return The initializers map. - */ - public Map getInitializers() { - final Map clonedMap = new LinkedHashMap<>(); - for (final Map.Entry entry : initializers.entrySet()) { - clonedMap.put(entry.getKey(), new FunctionInitializer(entry.getValue())); - } - return clonedMap; + return installedClasses.get(mainClassName); } @Override diff -r 5ed57fe26f13 -r 248dc4f11e5b test/script/basic/JDK-8053905.js --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/script/basic/JDK-8053905.js Mon Apr 27 12:50:21 2015 +0200 @@ -0,0 +1,38 @@ +/* + * Copyright (c) 2010, 2014, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/** + * JDK-8053905: Eager code generation fails for earley boyer with split threshold set to 1000 + * + * @test + * @runif external.octane + * @fork + * @option -Dnashorn.compiler.splitter.threshold=1000 + * @option -scripting + * @option --lazy-compilation=false + */ + +var fn = __DIR__ + 'compile-octane.js'; +arguments.push("earley-boyer"); // run only earley-boyer +var url = new java.io.File(fn).toURL(); +load(url); diff -r 5ed57fe26f13 -r 248dc4f11e5b test/script/basic/JDK-8053905.js.EXPECTED --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/script/basic/JDK-8053905.js.EXPECTED Mon Apr 27 12:50:21 2015 +0200 @@ -0,0 +1,2 @@ +Compiling 'earley-boyer'... +Done. diff -r 5ed57fe26f13 -r 248dc4f11e5b test/script/basic/compile-octane-splitter.js --- a/test/script/basic/compile-octane-splitter.js Mon Apr 27 12:27:33 2015 +0200 +++ b/test/script/basic/compile-octane-splitter.js Mon Apr 27 12:50:21 2015 +0200 @@ -29,11 +29,10 @@ * forever, so make this test future safe, we specify them explicitly * * @test + * @runif external.octane * @fork + * @option -scripting * @option -Dnashorn.compiler.splitter.threshold=1000 - * @fork - * @runif external.octane - * @option -scripting * @option -Dnashorn.typeInfo.disabled=true * @option --class-cache-size=0 * @option --persistent-code-cache=false