8057825: Bug in apply specialization - if an apply specialization that is available doesn't fit, a new one wouldn't be installed, if the new code generated as a specialization didn't manage to do the apply specialization. Basically changing a conditional to an unconditional.
Reviewed-by: attila, hannesw
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/ApplySpecialization.java Mon Nov 03 14:59:34 2014 +0100
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/ApplySpecialization.java Wed Nov 05 12:34:06 2014 +0100
@@ -29,6 +29,7 @@
import static jdk.nashorn.internal.codegen.CompilerConstants.EXPLODED_ARGUMENT_PREFIX;
import java.lang.invoke.MethodType;
+import java.net.URL;
import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Deque;
@@ -93,6 +94,8 @@
private final Deque<List<IdentNode>> explodedArguments = new ArrayDeque<>();
+ private final Deque<MethodType> callSiteTypes = new ArrayDeque<>();
+
private static final String ARGUMENTS = ARGUMENTS_VAR.symbolName();
/**
@@ -118,86 +121,108 @@
return context.getLogger(this.getClass());
}
+ @SuppressWarnings("serial")
+ private static class TransformFailedException extends RuntimeException {
+ TransformFailedException(final FunctionNode fn, final String message) {
+ super(massageURL(fn.getSource().getURL()) + '.' + fn.getName() + " => " + message, null, false, false);
+ }
+ }
+
+ @SuppressWarnings("serial")
+ private static class AppliesFoundException extends RuntimeException {
+ AppliesFoundException() {
+ super("applies_found", null, false, false);
+ }
+ }
+
+ private static final AppliesFoundException HAS_APPLIES = new AppliesFoundException();
+
+ private boolean hasApplies(final FunctionNode functionNode) {
+ try {
+ functionNode.accept(new NodeVisitor<LexicalContext>(new LexicalContext()) {
+ @Override
+ public boolean enterCallNode(final CallNode callNode) {
+ if (isApply(callNode)) {
+ throw HAS_APPLIES;
+ }
+ return true;
+ }
+ });
+ } catch (final AppliesFoundException e) {
+ return true;
+ }
+
+ log.fine("There are no applies in ", DebugLogger.quote(functionNode.getName()), " - nothing to do.");
+ return false; // no applies
+ }
+
/**
* Arguments may only be used as args to the apply. Everything else is disqualified
* We cannot control arguments if they escape from the method and go into an unknown
* scope, thus we are conservative and treat any access to arguments outside the
* apply call as a case of "we cannot apply the optimization".
- *
- * @return true if arguments escape
*/
- private boolean argumentsEscape(final FunctionNode functionNode) {
-
- @SuppressWarnings("serial")
- final UnsupportedOperationException uoe = new UnsupportedOperationException() {
- @Override
- public synchronized Throwable fillInStackTrace() {
- return null;
- }
- };
+ private void checkValidTransform(final FunctionNode functionNode) {
final Set<Expression> argumentsFound = new HashSet<>();
final Deque<Set<Expression>> stack = new ArrayDeque<>();
+
//ensure that arguments is only passed as arg to apply
- try {
- functionNode.accept(new NodeVisitor<LexicalContext>(new LexicalContext()) {
- private boolean isCurrentArg(final Expression expr) {
- return !stack.isEmpty() && stack.peek().contains(expr); //args to current apply call
- }
+ functionNode.accept(new NodeVisitor<LexicalContext>(new LexicalContext()) {
+
+ private boolean isCurrentArg(final Expression expr) {
+ return !stack.isEmpty() && stack.peek().contains(expr); //args to current apply call
+ }
- private boolean isArguments(final Expression expr) {
- if (expr instanceof IdentNode && ARGUMENTS.equals(((IdentNode)expr).getName())) {
- argumentsFound.add(expr);
+ private boolean isArguments(final Expression expr) {
+ if (expr instanceof IdentNode && ARGUMENTS.equals(((IdentNode)expr).getName())) {
+ argumentsFound.add(expr);
+ return true;
+ }
+ return false;
+ }
+
+ private boolean isParam(final String name) {
+ for (final IdentNode param : functionNode.getParameters()) {
+ if (param.getName().equals(name)) {
return true;
}
- return false;
}
+ return false;
+ }
- private boolean isParam(final String name) {
- for (final IdentNode param : functionNode.getParameters()) {
- if (param.getName().equals(name)) {
- return true;
- }
- }
- return false;
+ @Override
+ public Node leaveIdentNode(final IdentNode identNode) {
+ if (isParam(identNode.getName())) {
+ throw new TransformFailedException(lc.getCurrentFunction(), "parameter: " + identNode.getName());
}
-
- @Override
- public Node leaveIdentNode(final IdentNode identNode) {
- if (isParam(identNode.getName()) || isArguments(identNode) && !isCurrentArg(identNode)) {
- throw uoe; //avoid filling in stack trace
- }
- return identNode;
+ // it's OK if 'argument' occurs as the current argument of an apply
+ if (isArguments(identNode) && !isCurrentArg(identNode)) {
+ throw new TransformFailedException(lc.getCurrentFunction(), "is 'arguments': " + identNode.getName());
}
+ return identNode;
+ }
- @Override
- public boolean enterCallNode(final CallNode callNode) {
- final Set<Expression> callArgs = new HashSet<>();
- if (isApply(callNode)) {
- final List<Expression> argList = callNode.getArgs();
- if (argList.size() != 2 || !isArguments(argList.get(argList.size() - 1))) {
- throw new UnsupportedOperationException();
- }
- callArgs.addAll(callNode.getArgs());
+ @Override
+ public boolean enterCallNode(final CallNode callNode) {
+ final Set<Expression> callArgs = new HashSet<>();
+ if (isApply(callNode)) {
+ final List<Expression> argList = callNode.getArgs();
+ if (argList.size() != 2 || !isArguments(argList.get(argList.size() - 1))) {
+ throw new TransformFailedException(lc.getCurrentFunction(), "argument pattern not matched: " + argList);
}
- stack.push(callArgs);
- return true;
+ callArgs.addAll(callNode.getArgs());
}
+ stack.push(callArgs);
+ return true;
+ }
- @Override
- public Node leaveCallNode(final CallNode callNode) {
- stack.pop();
- return callNode;
- }
- });
- } catch (final UnsupportedOperationException e) {
- if (!argumentsFound.isEmpty()) {
- log.fine("'arguments' is used but escapes, or is reassigned in '" + functionNode.getName() + "'. Aborting");
+ @Override
+ public Node leaveCallNode(final CallNode callNode) {
+ stack.pop();
+ return callNode;
}
- return true; //bad
- }
-
- return false;
+ });
}
@Override
@@ -224,12 +249,14 @@
final CallNode newCallNode = callNode.setArgs(newArgs).setIsApplyToCall();
- log.fine("Transformed ",
- callNode,
- " from apply to call => ",
- newCallNode,
- " in ",
- DebugLogger.quote(lc.getCurrentFunction().getName()));
+ if (log.isEnabled()) {
+ log.fine("Transformed ",
+ callNode,
+ " from apply to call => ",
+ newCallNode,
+ " in ",
+ DebugLogger.quote(lc.getCurrentFunction().getName()));
+ }
return newCallNode;
}
@@ -237,12 +264,12 @@
return callNode;
}
- private boolean pushExplodedArgs(final FunctionNode functionNode) {
+ private void pushExplodedArgs(final FunctionNode functionNode) {
int start = 0;
final MethodType actualCallSiteType = compiler.getCallSiteType(functionNode);
if (actualCallSiteType == null) {
- return false;
+ throw new TransformFailedException(lc.getCurrentFunction(), "No callsite type");
}
assert actualCallSiteType.parameterType(actualCallSiteType.parameterCount() - 1) != Object[].class : "error vararg callsite passed to apply2call " + functionNode.getName() + " " + actualCallSiteType;
@@ -264,8 +291,8 @@
}
}
+ callSiteTypes.push(actualCallSiteType);
explodedArguments.push(newParams);
- return true;
}
@Override
@@ -288,11 +315,30 @@
return false;
}
- if (argumentsEscape(functionNode)) {
+ if (!hasApplies(functionNode)) {
return false;
}
- return pushExplodedArgs(functionNode);
+ if (log.isEnabled()) {
+ log.info("Trying to specialize apply to call in '",
+ functionNode.getName(),
+ "' params=",
+ functionNode.getParameters(),
+ " id=",
+ functionNode.getId(),
+ " source=",
+ massageURL(functionNode.getSource().getURL()));
+ }
+
+ try {
+ checkValidTransform(functionNode);
+ pushExplodedArgs(functionNode);
+ } catch (final TransformFailedException e) {
+ log.info("Failure: ", e.getMessage());
+ return false;
+ }
+
+ return true;
}
/**
@@ -300,8 +346,8 @@
* @return true if successful, false otherwise
*/
@Override
- public Node leaveFunctionNode(final FunctionNode functionNode0) {
- FunctionNode newFunctionNode = functionNode0;
+ public Node leaveFunctionNode(final FunctionNode functionNode) {
+ FunctionNode newFunctionNode = functionNode;
final String functionName = newFunctionNode.getName();
if (changed.contains(newFunctionNode.getId())) {
@@ -310,17 +356,18 @@
setParameters(lc, explodedArguments.peek());
if (log.isEnabled()) {
- log.info("Successfully specialized apply to call in '",
+ log.info("Success: ",
+ massageURL(newFunctionNode.getSource().getURL()),
+ '.',
functionName,
- " params=",
- explodedArguments.peek(),
"' id=",
newFunctionNode.getId(),
- " source=",
- newFunctionNode.getSource().getURL());
+ " params=",
+ callSiteTypes.peek());
}
}
+ callSiteTypes.pop();
explodedArguments.pop();
return newFunctionNode.setState(lc, CompilationState.BUILTINS_TRANSFORMED);
@@ -331,4 +378,15 @@
return f instanceof AccessNode && "apply".equals(((AccessNode)f).getProperty());
}
+ private static String massageURL(final URL url) {
+ if (url == null) {
+ return "<null>";
+ }
+ final String str = url.toString();
+ final int slash = str.lastIndexOf('/');
+ if (slash == -1) {
+ return str;
+ }
+ return str.substring(slash + 1);
+ }
}
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/CodeGenerator.java Mon Nov 03 14:59:34 2014 +0100
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/CodeGenerator.java Wed Nov 05 12:34:06 2014 +0100
@@ -615,7 +615,6 @@
static final TypeBounds UNBOUNDED = new TypeBounds(Type.UNKNOWN, Type.OBJECT);
static final TypeBounds INT = exact(Type.INT);
- static final TypeBounds NUMBER = exact(Type.NUMBER);
static final TypeBounds OBJECT = exact(Type.OBJECT);
static final TypeBounds BOOLEAN = exact(Type.BOOLEAN);
@@ -3894,7 +3893,7 @@
private static boolean isRhsZero(final BinaryNode binaryNode) {
final Expression rhs = binaryNode.rhs();
- return rhs instanceof LiteralNode && INT_ZERO.equals(((LiteralNode)rhs).getValue());
+ return rhs instanceof LiteralNode && INT_ZERO.equals(((LiteralNode<?>)rhs).getValue());
}
private void loadBIT_XOR(final BinaryNode binaryNode) {
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/types/Type.java Mon Nov 03 14:59:34 2014 +0100
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/types/Type.java Wed Nov 05 12:34:06 2014 +0100
@@ -1155,6 +1155,10 @@
return type;
}
+ /**
+ * Read resolve
+ * @return resolved type
+ */
protected final Object readResolve() {
return Type.typeFor(clazz);
}
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/objects/NativeString.java Mon Nov 03 14:59:34 2014 +0100
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/objects/NativeString.java Wed Nov 05 12:34:06 2014 +0100
@@ -29,6 +29,7 @@
import static jdk.nashorn.internal.runtime.ECMAErrors.typeError;
import static jdk.nashorn.internal.runtime.JSType.isRepresentableAsInt;
import static jdk.nashorn.internal.runtime.ScriptRuntime.UNDEFINED;
+
import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
import java.lang.invoke.MethodType;
@@ -1380,7 +1381,6 @@
* sequence and that we are in range
*/
private static final class CharCodeAtLinkLogic extends SpecializedFunction.LinkLogic {
-
private static final CharCodeAtLinkLogic INSTANCE = new CharCodeAtLinkLogic();
@Override
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/RecompilableScriptFunctionData.java Mon Nov 03 14:59:34 2014 +0100
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/RecompilableScriptFunctionData.java Wed Nov 05 12:34:06 2014 +0100
@@ -26,6 +26,7 @@
package jdk.nashorn.internal.runtime;
import static jdk.nashorn.internal.lookup.Lookup.MH;
+
import java.io.IOException;
import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
@@ -727,7 +728,7 @@
assert existingBest != null;
//we are calling a vararg method with real args
- boolean applyToCall = existingBest.isVarArg() && !CompiledFunction.isVarArgsType(callSiteType);
+ 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
@@ -736,14 +737,16 @@
if (best != null) {
return best;
}
- applyToCall = true;
+ varArgWithRealArgs = true;
}
- if (applyToCall) {
+ 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);
- if ((fnInit.getFlags() & FunctionNode.HAS_APPLY_TO_CALL_SPECIALIZATION) != 0) { //did the specialization work
- existingBest = addCode(fnInit, callSiteType);
- }
+ existingBest = addCode(fnInit, callSiteType);
}
return existingBest;
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/arrays/SparseArrayData.java Mon Nov 03 14:59:34 2014 +0100
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/arrays/SparseArrayData.java Wed Nov 05 12:34:06 2014 +0100
@@ -36,12 +36,7 @@
* Handle arrays where the index is very large.
*/
class SparseArrayData extends ArrayData {
- static final long MAX_DENSE_LENGTH = 16 * 512 * 1024;
-
- static {
- // we must break into sparse arrays before we require long indexes
- assert MAX_DENSE_LENGTH <= Integer.MAX_VALUE;
- }
+ static final int MAX_DENSE_LENGTH = 8 * 1024 * 1024;
/** Underlying array. */
private ArrayData underlying;
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/events/RecompilationEvent.java Mon Nov 03 14:59:34 2014 +0100
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/events/RecompilationEvent.java Wed Nov 05 12:34:06 2014 +0100
@@ -47,7 +47,7 @@
* @param level logging level
* @param rewriteException rewriteException wrapped by this RuntimEvent
* @param returnValue rewriteException return value - as we don't want to make
- * {@link RewriteException#getReturnValueNonDestructive()} public, we pass it as
+ * {@code RewriteException.getReturnValueNonDestructive()} public, we pass it as
* an extra parameter, rather than querying the getter from another package.
*/
public RecompilationEvent(final Level level, final RewriteException rewriteException, final Object returnValue) {
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/JDK-8057825.js Wed Nov 05 12:34:06 2014 +0100
@@ -0,0 +1,45 @@
+/*
+ * 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-8057825 : A failed apply to call generation should NOT reuse the
+ * best apply to call generation so far - because it may not fit!!!
+ *
+ * @test
+ * @run
+ */
+
+function createApplier(f) {
+ function applier() {
+ f.apply(this, arguments); // no transform applied here
+ }
+ return applier;
+}
+
+function printer(x,y) {
+ print(x + " and " + y);
+}
+
+var printerApplier = createApplier(printer);
+printerApplier();
+printerApplier.apply(this, ["foo", "bar"]);
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/JDK-8057825.js.EXPECTED Wed Nov 05 12:34:06 2014 +0100
@@ -0,0 +1,2 @@
+undefined and undefined
+foo and bar
--- a/nashorn/test/src/jdk/internal/dynalink/beans/CallerSensitiveTest.java Mon Nov 03 14:59:34 2014 +0100
+++ b/nashorn/test/src/jdk/internal/dynalink/beans/CallerSensitiveTest.java Wed Nov 05 12:34:06 2014 +0100
@@ -28,6 +28,7 @@
import jdk.nashorn.test.models.ClassLoaderAware;
import org.testng.annotations.Test;
+@SuppressWarnings("javadoc")
public class CallerSensitiveTest {
@Test
public void testCallerSensitive() {
--- a/nashorn/test/src/jdk/nashorn/test/models/ClassLoaderAware.java Mon Nov 03 14:59:34 2014 +0100
+++ b/nashorn/test/src/jdk/nashorn/test/models/ClassLoaderAware.java Wed Nov 05 12:34:06 2014 +0100
@@ -25,6 +25,7 @@
package jdk.nashorn.test.models;
+@SuppressWarnings("javadoc")
public interface ClassLoaderAware {
public ClassLoader getContextClassLoader();
public void checkMemberAccess(Class<?> clazz, int which);
--- a/nashorn/test/src/jdk/nashorn/test/models/NullProvider.java Mon Nov 03 14:59:34 2014 +0100
+++ b/nashorn/test/src/jdk/nashorn/test/models/NullProvider.java Wed Nov 05 12:34:06 2014 +0100
@@ -25,7 +25,7 @@
package jdk.nashorn.test.models;
-
+@SuppressWarnings("javadoc")
public class NullProvider {
public static Integer getInteger() { return null; }
public static Long getLong() { return null; }