stan/jdk-24
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

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

Browse Source 
Reviewed-by: attila, hannesw
This commit is contained in:
Marcus Lagergren 2014-11-05 12:34:06 +01:00
parent d82a461e44
commit fe0da815c5
12 changed files with 208 additions and 100 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

224
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/ApplySpecialization.java
View File

@ -29,6 +29,7 @@ import static jdk.nashorn.internal.codegen.CompilerConstants.ARGUMENTS_VAR;
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 @@ public final class ApplySpecialization extends NodeVisitor<LexicalContext> imple
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 @@ public final class ApplySpecialization extends NodeVisitor<LexicalContext> imple
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
}
private boolean isArguments(final Expression expr) {
if (expr instanceof IdentNode && ARGUMENTS.equals(((IdentNode)expr).getName())) {
argumentsFound.add(expr);
//ensure that arguments is only passed as arg to apply
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);
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;
}
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()) || isArguments(identNode) && !isCurrentArg(identNode)) {
throw uoe; //avoid filling in stack trace
}
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());
}
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");
return false;
}
return true; //bad
}
return false;
@Override
public Node leaveIdentNode(final IdentNode identNode) {
if (isParam(identNode.getName())) {
throw new TransformFailedException(lc.getCurrentFunction(), "parameter: " + identNode.getName());
}
// 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 TransformFailedException(lc.getCurrentFunction(), "argument pattern not matched: " + argList);
}
callArgs.addAll(callNode.getArgs());
}
stack.push(callArgs);
return true;
}
@Override
public Node leaveCallNode(final CallNode callNode) {
stack.pop();
return callNode;
}
});
}
@Override
@ -224,12 +249,14 @@ public final class ApplySpecialization extends NodeVisitor<LexicalContext> imple
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 @@ public final class ApplySpecialization extends NodeVisitor<LexicalContext> imple
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 @@ public final class ApplySpecialization extends NodeVisitor<LexicalContext> imple
}
}
callSiteTypes.push(actualCallSiteType);
explodedArguments.push(newParams);
return true;
}
@Override
@ -288,11 +315,30 @@ public final class ApplySpecialization extends NodeVisitor<LexicalContext> imple
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 @@ public final class ApplySpecialization extends NodeVisitor<LexicalContext> imple
* @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 @@ public final class ApplySpecialization extends NodeVisitor<LexicalContext> imple
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 @@ public final class ApplySpecialization extends NodeVisitor<LexicalContext> imple
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);
}
}

3
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/CodeGenerator.java
View File

@ -615,7 +615,6 @@ final class CodeGenerator extends NodeOperatorVisitor<CodeGeneratorLexicalContex
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 @@ final class CodeGenerator extends NodeOperatorVisitor<CodeGeneratorLexicalContex
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) {

4
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/types/Type.java
View File

@ -1155,6 +1155,10 @@ public abstract class Type implements Comparable<Type>, BytecodeOps, Serializabl
return type;
}
/**
* Read resolve
* @return resolved type
*/
protected final Object readResolve() {
return Type.typeFor(clazz);
}

2
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/objects/NativeString.java
View File

@ -29,6 +29,7 @@ import static jdk.nashorn.internal.lookup.Lookup.MH;
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 @@ public final class NativeString extends ScriptObject implements OptimisticBuilti
* sequence and that we are in range
*/
private static final class CharCodeAtLinkLogic extends SpecializedFunction.LinkLogic {
private static final CharCodeAtLinkLogic INSTANCE = new CharCodeAtLinkLogic();
@Override

15
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/RecompilableScriptFunctionData.java
View File

@ -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 @@ public final class RecompilableScriptFunctionData extends ScriptFunctionData imp
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 @@ public final class RecompilableScriptFunctionData extends ScriptFunctionData imp
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;

7
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/arrays/SparseArrayData.java
View File

@ -36,12 +36,7 @@ import jdk.nashorn.internal.runtime.ScriptRuntime;
* 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;

2
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/events/RecompilationEvent.java
View File

@ -47,7 +47,7 @@ public final class RecompilationEvent extends RuntimeEvent<RewriteException> {
* @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) {

45
nashorn/test/script/basic/JDK-8057825.js Normal file
View File

@ -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"]);

2
nashorn/test/script/basic/JDK-8057825.js.EXPECTED Normal file
View File

@ -0,0 +1,2 @@
undefined and undefined
foo and bar

1
nashorn/test/src/jdk/internal/dynalink/beans/CallerSensitiveTest.java
View File

@ -28,6 +28,7 @@ package jdk.internal.dynalink.beans;
import jdk.nashorn.test.models.ClassLoaderAware;
import org.testng.annotations.Test;
@SuppressWarnings("javadoc")
public class CallerSensitiveTest {
@Test
public void testCallerSensitive() {

1
nashorn/test/src/jdk/nashorn/test/models/ClassLoaderAware.java
View File

@ -25,6 +25,7 @@
package jdk.nashorn.test.models;
@SuppressWarnings("javadoc")
public interface ClassLoaderAware {
public ClassLoader getContextClassLoader();
public void checkMemberAccess(Class<?> clazz, int which);

2
nashorn/test/src/jdk/nashorn/test/models/NullProvider.java
View File

@ -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; }