changeset 1358:348ce347ba14

8131340: Varargs function is recompiled each time it is linked Reviewed-by: mhaupt, sundar
author hannesw
date Mon, 20 Jul 2015 13:11:26 +0200
parents 2e8bb16872d7
children b983e998f528
files src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/FinalScriptFunctionData.java src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/RecompilableScriptFunctionData.java src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/ScriptFunctionData.java test/script/basic/JDK-8131340.js
diffstat 4 files changed, 80 insertions(+), 40 deletions(-) [+]
line wrap: on
line diff
--- a/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/FinalScriptFunctionData.java	Thu Jul 16 19:30:19 2015 -0700
+++ b/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/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/RecompilableScriptFunctionData.java	Thu Jul 16 19:30:19 2015 -0700
+++ b/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/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/ScriptFunctionData.java	Thu Jul 16 19:30:19 2015 -0700
+++ b/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
+    abstract CompiledFunction getBest(final MethodType callSiteType, final ScriptObject runtimeScope, final Collection<CompiledFunction> forbidden);
 
-        if (isRecompilable()) {
-            final CompiledFunction candidate = pickFunction(callSiteType, false);
-            if (candidate != null) {
-                return candidate;
-            }
-            return pickFunction(callSiteType, true); //try vararg last
-        }
-
-        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/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);