8131340: Varargs function is recompiled each time it is linked
Reviewed-by: mhaupt, sundar
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/FinalScriptFunctionData.java Wed Jul 05 20:42:39 2017 +0200
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/FinalScriptFunctionData.java Mon Jul 20 13:11:26 2015 +0200
@@ -27,6 +27,7 @@
import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodType;
+import java.util.Collection;
import java.util.List;
/**
@@ -72,11 +73,6 @@
}
@Override
- boolean isRecompilable() {
- return false;
- }
-
- @Override
protected boolean needsCallee() {
final boolean needsCallee = code.getFirst().needsCallee();
assert allNeedCallee(needsCallee);
@@ -93,6 +89,20 @@
}
@Override
+ CompiledFunction getBest(final MethodType callSiteType, final ScriptObject runtimeScope, final Collection<CompiledFunction> forbidden) {
+ assert isValidCallSite(callSiteType) : callSiteType;
+
+ CompiledFunction best = null;
+ for (final CompiledFunction candidate: code) {
+ if (!forbidden.contains(candidate) && candidate.betterThanFinal(best, callSiteType)) {
+ best = candidate;
+ }
+ }
+
+ return best;
+ }
+
+ @Override
MethodType getGenericType() {
// We need to ask the code for its generic type. We can't just rely on this function data's arity, as it's not
// actually correct for lots of built-ins. E.g. ECMAScript 5.1 section 15.5.3.2 prescribes that
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/RecompilableScriptFunctionData.java Wed Jul 05 20:42:39 2017 +0200
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/RecompilableScriptFunctionData.java Mon Jul 20 13:11:26 2015 +0200
@@ -617,6 +617,7 @@
private CompiledFunction addCode(final MethodHandle target, final Map<Integer, Type> invalidatedProgramPoints,
final MethodType callSiteType, final int fnFlags) {
final CompiledFunction cfn = new CompiledFunction(target, this, invalidatedProgramPoints, callSiteType, fnFlags);
+ assert noDuplicateCode(cfn) : "duplicate code";
code.add(cfn);
return cfn;
}
@@ -683,14 +684,17 @@
@Override
synchronized CompiledFunction getBest(final MethodType callSiteType, final ScriptObject runtimeScope, final Collection<CompiledFunction> forbidden) {
- CompiledFunction existingBest = super.getBest(callSiteType, runtimeScope, forbidden);
+ assert isValidCallSite(callSiteType) : callSiteType;
+
+ CompiledFunction existingBest = pickFunction(callSiteType, false);
+ if (existingBest == null) {
+ existingBest = pickFunction(callSiteType, true); // try vararg last
+ }
if (existingBest == null) {
existingBest = addCode(compileTypeSpecialization(callSiteType, runtimeScope, true), callSiteType);
}
assert existingBest != null;
- //we are calling a vararg method with real args
- boolean varArgWithRealArgs = existingBest.isVarArg() && !CompiledFunction.isVarArgsType(callSiteType);
//if the best one is an apply to call, it has to match the callsite exactly
//or we need to regenerate
@@ -699,27 +703,18 @@
if (best != null) {
return best;
}
- varArgWithRealArgs = true;
- }
- if (varArgWithRealArgs) {
// special case: we had an apply to call, but we failed to make it fit.
// Try to generate a specialized one for this callsite. It may
// be another apply to call specialization, or it may not, but whatever
// it is, it is a specialization that is guaranteed to fit
- final FunctionInitializer fnInit = compileTypeSpecialization(callSiteType, runtimeScope, false);
- existingBest = addCode(fnInit, callSiteType);
+ existingBest = addCode(compileTypeSpecialization(callSiteType, runtimeScope, false), callSiteType);
}
return existingBest;
}
@Override
- boolean isRecompilable() {
- return true;
- }
-
- @Override
public boolean needsCallee() {
return getFunctionFlag(FunctionNode.NEEDS_CALLEE);
}
@@ -827,6 +822,16 @@
return newFn;
}
+ // Make sure code does not contain a compiled function with the same signature as compiledFunction
+ private boolean noDuplicateCode(final CompiledFunction compiledFunction) {
+ for (final CompiledFunction cf : code) {
+ if (cf.type().equals(compiledFunction.type())) {
+ return false;
+ }
+ }
+ return true;
+ }
+
private void readObject(final java.io.ObjectInputStream in) throws IOException, ClassNotFoundException {
in.defaultReadObject();
createLogger();
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/ScriptFunctionData.java Wed Jul 05 20:42:39 2017 +0200
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/ScriptFunctionData.java Mon Jul 20 13:11:26 2015 +0200
@@ -359,31 +359,13 @@
* scope is not known, but that might cause compilation of code that will need more deoptimization passes.
* @return the best function for the specified call site type.
*/
- CompiledFunction getBest(final MethodType callSiteType, final ScriptObject runtimeScope, final Collection<CompiledFunction> forbidden) {
- assert callSiteType.parameterCount() >= 2 : callSiteType; // Must have at least (callee, this)
- assert callSiteType.parameterType(0).isAssignableFrom(ScriptFunction.class) : callSiteType; // Callee must be assignable from script function
-
- if (isRecompilable()) {
- final CompiledFunction candidate = pickFunction(callSiteType, false);
- if (candidate != null) {
- return candidate;
- }
- return pickFunction(callSiteType, true); //try vararg last
- }
+ abstract CompiledFunction getBest(final MethodType callSiteType, final ScriptObject runtimeScope, final Collection<CompiledFunction> forbidden);
- CompiledFunction best = null;
- for (final CompiledFunction candidate: code) {
- if (!forbidden.contains(candidate) && candidate.betterThanFinal(best, callSiteType)) {
- best = candidate;
- }
- }
-
- return best;
+ boolean isValidCallSite(final MethodType callSiteType) {
+ return callSiteType.parameterCount() >= 2 && // Must have at least (callee, this)
+ callSiteType.parameterType(0).isAssignableFrom(ScriptFunction.class); // Callee must be assignable from script function
}
-
- abstract boolean isRecompilable();
-
CompiledFunction getGeneric(final ScriptObject runtimeScope) {
return getBest(getGenericType(), runtimeScope, CompiledFunction.NO_FUNCTIONS);
}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/JDK-8131340.js Mon Jul 20 13:11:26 2015 +0200
@@ -0,0 +1,43 @@
+/*
+ * Copyright (c) 2015, 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-8131340: Varargs function is recompiled each time it is linked
+ *
+ * @test
+ * @run
+ */
+
+// This is an indirect test. If repeated calls were to cause recompilation
+// this would trigger an assertion in RecompilableScriptFunctionData.
+
+function varargs() {
+ return arguments;
+}
+
+varargs(1);
+varargs(2);
+varargs(3);
+varargs(4);
+varargs(5);
+varargs(6);