8170322: Specialized functions convert booleans to numbers
authorhannesw
Fri, 25 Nov 2016 14:20:24 +0100
changeset 42376 8604f1a50c30
parent 42279 f4e854a77aa3
child 42377 0bbd5e43e671
8170322: Specialized functions convert booleans to numbers Reviewed-by: jlaskey, attila
nashorn/buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/MemberInfo.java
nashorn/buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/MethodGenerator.java
nashorn/buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/ScriptClassInfoCollector.java
nashorn/buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/StringConstants.java
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/objects/NativeArray.java
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/objects/NativeString.java
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/objects/annotations/SpecializedFunction.java
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/CompiledFunction.java
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/Specialization.java
nashorn/test/script/basic/JDK-8170322.js
--- a/nashorn/buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/MemberInfo.java	Wed Jul 05 22:30:52 2017 +0200
+++ b/nashorn/buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/MemberInfo.java	Fri Nov 25 14:20:24 2016 +0100
@@ -29,7 +29,6 @@
 import static jdk.nashorn.internal.tools.nasgen.StringConstants.OBJ_PKG;
 import static jdk.nashorn.internal.tools.nasgen.StringConstants.RUNTIME_PKG;
 import static jdk.nashorn.internal.tools.nasgen.StringConstants.SCRIPTS_PKG;
-import static jdk.nashorn.internal.tools.nasgen.StringConstants.SCRIPTOBJECT_DESC;
 import static jdk.nashorn.internal.tools.nasgen.StringConstants.STRING_DESC;
 import static jdk.nashorn.internal.tools.nasgen.StringConstants.TYPE_SYMBOL;
 
@@ -111,6 +110,8 @@
 
     private boolean isOptimistic;
 
+    private boolean convertsNumericArgs;
+
     /**
      * @return the kind
      */
@@ -172,6 +173,23 @@
     }
 
     /**
+     * Check if this function converts arguments for numeric parameters to numbers
+     * so it's safe to pass booleans as 0 and 1
+     * @return true if it is safe to convert arguments to numbers
+     */
+    public boolean convertsNumericArgs() {
+        return convertsNumericArgs;
+    }
+
+    /**
+     * Tag this as a function that converts arguments for numeric params to numbers
+     * @param convertsNumericArgs if true args can be safely converted to numbers
+     */
+    public void setConvertsNumericArgs(final boolean convertsNumericArgs) {
+        this.convertsNumericArgs = convertsNumericArgs;
+    }
+
+    /**
      * Get the SpecializedFunction guard for specializations, i.e. optimistic
      * builtins
      * @return specialization, null if none
--- a/nashorn/buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/MethodGenerator.java	Wed Jul 05 22:30:52 2017 +0200
+++ b/nashorn/buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/MethodGenerator.java	Fri Nov 25 14:20:24 2016 +0100
@@ -412,6 +412,7 @@
                 visitLdcInsn(linkLogicClass);
             }
             visitInsn(mi.isOptimistic() ? ICONST_1 : ICONST_0);
+            visitInsn(mi.convertsNumericArgs() ? ICONST_1 : ICONST_0);
             visitMethodInsn(INVOKESPECIAL, SPECIALIZATION_TYPE, INIT, ctor, false);
             arrayStore(TYPE_SPECIALIZATION);
         }
--- a/nashorn/buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/ScriptClassInfoCollector.java	Wed Jul 05 22:30:52 2017 +0200
+++ b/nashorn/buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/ScriptClassInfoCollector.java	Fri Nov 25 14:20:24 2016 +0100
@@ -210,6 +210,7 @@
                         private Where   where;
                         private boolean isSpecializedConstructor;
                         private boolean isOptimistic;
+                        private boolean convertsNumericArgs;
                         private Type    linkLogicClass = MethodGenerator.EMPTY_LINK_LOGIC_TYPE;
 
                         @Override
@@ -238,6 +239,10 @@
                             case "linkLogic":
                                 this.linkLogicClass = (Type)annotationValue;
                                 break;
+                            case "convertsNumericArgs":
+                                assert annoKind == Kind.SPECIALIZED_FUNCTION;
+                                this.convertsNumericArgs = (Boolean)annotationValue;
+                                break;
                             default:
                                 break;
                             }
@@ -298,6 +303,7 @@
                             memInfo.setLinkLogicClass(linkLogicClass);
                             memInfo.setIsSpecializedConstructor(isSpecializedConstructor);
                             memInfo.setIsOptimistic(isOptimistic);
+                            memInfo.setConvertsNumericArgs(convertsNumericArgs);
                         }
                     };
                 }
--- a/nashorn/buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/StringConstants.java	Wed Jul 05 22:30:52 2017 +0200
+++ b/nashorn/buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/StringConstants.java	Fri Nov 25 14:20:24 2016 +0100
@@ -45,7 +45,6 @@
 
     // standard jdk types, methods
     static final Type TYPE_METHODHANDLE         = Type.getType(MethodHandle.class);
-    static final Type TYPE_METHODHANDLE_ARRAY   = Type.getType(MethodHandle[].class);
     static final Type TYPE_SPECIALIZATION       = Type.getType("L" + RUNTIME_PKG + "Specialization;");
     static final Type TYPE_SPECIALIZATION_ARRAY = Type.getType("[L" + RUNTIME_PKG + "Specialization;");
     static final Type TYPE_OBJECT               = Type.getType(Object.class);
@@ -60,13 +59,11 @@
     static final String INIT = "<init>";
     static final String DEFAULT_INIT_DESC = Type.getMethodDescriptor(Type.VOID_TYPE);
 
-    static final String METHODHANDLE_TYPE = TYPE_METHODHANDLE.getInternalName();
     static final String SPECIALIZATION_TYPE = TYPE_SPECIALIZATION.getInternalName();
-    static final String SPECIALIZATION_INIT2 = Type.getMethodDescriptor(Type.VOID_TYPE, TYPE_METHODHANDLE, Type.getType(boolean.class));
-    static final String SPECIALIZATION_INIT3 = Type.getMethodDescriptor(Type.VOID_TYPE, TYPE_METHODHANDLE, TYPE_CLASS, Type.getType(boolean.class));
+    static final String SPECIALIZATION_INIT2 = Type.getMethodDescriptor(Type.VOID_TYPE, TYPE_METHODHANDLE, Type.BOOLEAN_TYPE, Type.BOOLEAN_TYPE);
+    static final String SPECIALIZATION_INIT3 = Type.getMethodDescriptor(Type.VOID_TYPE, TYPE_METHODHANDLE, TYPE_CLASS, Type.BOOLEAN_TYPE, Type.BOOLEAN_TYPE);
     static final String OBJECT_TYPE = TYPE_OBJECT.getInternalName();
     static final String OBJECT_DESC = TYPE_OBJECT.getDescriptor();
-    static final String STRING_TYPE = TYPE_STRING.getInternalName();
     static final String STRING_DESC = TYPE_STRING.getDescriptor();
     static final String OBJECT_ARRAY_DESC = Type.getDescriptor(Object[].class);
     static final String ARRAYLIST_TYPE = TYPE_ARRAYLIST.getInternalName();
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/objects/NativeArray.java	Wed Jul 05 22:30:52 2017 +0200
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/objects/NativeArray.java	Fri Nov 25 14:20:24 2016 +0100
@@ -61,7 +61,6 @@
 import jdk.nashorn.internal.runtime.OptimisticBuiltins;
 import jdk.nashorn.internal.runtime.PropertyDescriptor;
 import jdk.nashorn.internal.runtime.PropertyMap;
-import jdk.nashorn.internal.runtime.ScriptFunction;
 import jdk.nashorn.internal.runtime.ScriptObject;
 import jdk.nashorn.internal.runtime.ScriptRuntime;
 import jdk.nashorn.internal.runtime.Undefined;
@@ -748,7 +747,7 @@
      * @param arg argument
      * @return resulting NativeArray
      */
-    @SpecializedFunction(linkLogic=ConcatLinkLogic.class)
+    @SpecializedFunction(linkLogic=ConcatLinkLogic.class, convertsNumericArgs = false)
     public static NativeArray concat(final Object self, final int arg) {
         final ContinuousArrayData newData = getContinuousArrayDataCCE(self, Integer.class).copy(); //get at least an integer data copy of this data
         newData.fastPush(arg); //add an integer to its end
@@ -762,21 +761,7 @@
      * @param arg argument
      * @return resulting NativeArray
      */
-    @SpecializedFunction(linkLogic=ConcatLinkLogic.class)
-    public static NativeArray concat(final Object self, final long arg) {
-        final ContinuousArrayData newData = getContinuousArrayDataCCE(self, Long.class).copy(); //get at least a long array data copy of this data
-        newData.fastPush(arg); //add a long at the end
-        return new NativeArray(newData);
-    }
-
-    /**
-     * ECMA 15.4.4.4 Array.prototype.concat ( [ item1 [ , item2 [ , ... ] ] ] )
-     *
-     * @param self self reference
-     * @param arg argument
-     * @return resulting NativeArray
-     */
-    @SpecializedFunction(linkLogic=ConcatLinkLogic.class)
+    @SpecializedFunction(linkLogic=ConcatLinkLogic.class, convertsNumericArgs = false)
     public static NativeArray concat(final Object self, final double arg) {
         final ContinuousArrayData newData = getContinuousArrayDataCCE(self, Double.class).copy(); //get at least a number array data copy of this data
         newData.fastPush(arg); //add a double at the end
@@ -980,7 +965,7 @@
      * @param arg a primitive to push
      * @return array length after push
      */
-    @SpecializedFunction(linkLogic=PushLinkLogic.class)
+    @SpecializedFunction(linkLogic=PushLinkLogic.class, convertsNumericArgs = false)
     public static double push(final Object self, final int arg) {
         return getContinuousArrayDataCCE(self, Integer.class).fastPush(arg);
     }
@@ -994,21 +979,7 @@
      * @param arg a primitive to push
      * @return array length after push
      */
-    @SpecializedFunction(linkLogic=PushLinkLogic.class)
-    public static double push(final Object self, final long arg) {
-        return getContinuousArrayDataCCE(self, Long.class).fastPush(arg);
-    }
-
-    /**
-     * ECMA 15.4.4.7 Array.prototype.push (args...)
-     *
-     * Primitive specialization, {@link LinkLogic}
-     *
-     * @param self self reference
-     * @param arg a primitive to push
-     * @return array length after push
-     */
-    @SpecializedFunction(linkLogic=PushLinkLogic.class)
+    @SpecializedFunction(linkLogic=PushLinkLogic.class, convertsNumericArgs = false)
     public static double push(final Object self, final double arg) {
         return getContinuousArrayDataCCE(self, Double.class).fastPush(arg);
     }
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/objects/NativeString.java	Wed Jul 05 22:30:52 2017 +0200
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/objects/NativeString.java	Fri Nov 25 14:20:24 2016 +0100
@@ -539,17 +539,6 @@
     }
 
     /**
-     * ECMA 15.5.4.5 String.prototype.charCodeAt (pos) - specialized version for long position
-     * @param self self reference
-     * @param pos  position in string
-     * @return number representing charcode at position
-     */
-    @SpecializedFunction(linkLogic=CharCodeAtLinkLogic.class)
-    public static int charCodeAt(final Object self, final long pos) {
-        return charCodeAt(self, (int)pos);
-    }
-
-    /**
      * ECMA 15.5.4.5 String.prototype.charCodeAt (pos) - specialized version for int position
      * @param self self reference
      * @param pos  position in string
@@ -1176,24 +1165,7 @@
     }
 
     /**
-     * ECMA 15.5.2.1 new String ( [ value ] ) - special version with exactly one {@code int} arg
-     *
-     * Constructor
-     *
-     * @param newObj is this constructor invoked with the new operator
-     * @param self   self reference
-     * @param arg    the arg
-     *
-     * @return new NativeString containing the string representation of the arg
-     */
-    @SpecializedFunction(isConstructor=true)
-    public static Object constructor(final boolean newObj, final Object self, final long arg) {
-        final String str = Long.toString(arg);
-        return newObj ? newObj(str) : str;
-    }
-
-    /**
-     * ECMA 15.5.2.1 new String ( [ value ] ) - special version with exactly one {@code int} arg
+     * ECMA 15.5.2.1 new String ( [ value ] ) - special version with exactly one {@code double} arg
      *
      * Constructor
      *
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/objects/annotations/SpecializedFunction.java	Wed Jul 05 22:30:52 2017 +0200
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/objects/annotations/SpecializedFunction.java	Fri Nov 25 14:20:24 2016 +0100
@@ -215,4 +215,14 @@
      * @return whether this function can throw {@link UnwarrantedOptimismException}.
      */
     boolean isOptimistic() default false;
+
+    /**
+     * Is it safe to convert non-numeric arguments to numbers for this function's primitive numeric parameters?
+     * This is true for many built-in functions which expect numeric arguments, but not for those that
+     * expect generic arguments and just have specializations with numeric params to avoid boxing overhead.
+     * The default value is {@code true} because that is by far the most common case.
+     *
+     * @return true if it is safe to convert arguments to numbers
+     */
+    boolean convertsNumericArgs() default true;
 }
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/CompiledFunction.java	Wed Jul 05 22:30:52 2017 +0200
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/CompiledFunction.java	Fri Nov 25 14:20:24 2016 +0100
@@ -152,6 +152,10 @@
         return null;
     }
 
+    boolean convertsNumericArgs() {
+        return isSpecialization() && specialization.convertsNumericArgs();
+    }
+
     int getFlags() {
         return flags;
     }
@@ -388,10 +392,18 @@
             int narrowWeightDelta = 0;
             int widenWeightDelta = 0;
             final int minParamsCount = Math.min(Math.min(thisParamCount, otherParamCount), callSiteParamCount);
+            final boolean convertsNumericArgs = cf.convertsNumericArgs();
             for(int i = 0; i < minParamsCount; ++i) {
-                final int callSiteParamWeight = getParamType(i, callSiteType, csVarArg).getWeight();
+                final Type callSiteParamType = getParamType(i, callSiteType, csVarArg);
+                final Type thisParamType = getParamType(i, thisType, thisVarArg);
+                if (!convertsNumericArgs && callSiteParamType.isBoolean() && thisParamType.isNumeric()) {
+                    // When an argument is converted to number by a function it is safe to "widen" booleans to numeric types.
+                    // However, we must avoid this conversion for generic functions such as Array.prototype.push.
+                    return false;
+                }
+                final int callSiteParamWeight = callSiteParamType.getWeight();
                 // Delta is negative for narrowing, positive for widening
-                final int thisParamWeightDelta = getParamType(i, thisType, thisVarArg).getWeight() - callSiteParamWeight;
+                final int thisParamWeightDelta = thisParamType.getWeight() - callSiteParamWeight;
                 final int otherParamWeightDelta = getParamType(i, otherType, otherVarArg).getWeight() - callSiteParamWeight;
                 // Only count absolute values of narrowings
                 narrowWeightDelta += Math.max(-thisParamWeightDelta, 0) - Math.max(-otherParamWeightDelta, 0);
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/Specialization.java	Wed Jul 05 22:30:52 2017 +0200
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/Specialization.java	Fri Nov 25 14:20:24 2016 +0100
@@ -36,6 +36,7 @@
     private final MethodHandle mh;
     private final Class<? extends LinkLogic> linkLogicClass;
     private final boolean isOptimistic;
+    private final boolean convertsNumericArgs;
 
     /**
      * Constructor
@@ -43,7 +44,7 @@
      * @param mh  invoker method handler
      */
     public Specialization(final MethodHandle mh) {
-        this(mh, false);
+        this(mh, false, true);
     }
 
     /**
@@ -52,9 +53,10 @@
      * @param mh  invoker method handler
      * @param isOptimistic is this an optimistic native method, i.e. can it throw {@link UnwarrantedOptimismException}
      *   which would have to lead to a relink and return value processing
+     * @param convertsNumericArgs true if it is safe to convert arguments to numbers
      */
-    public Specialization(final MethodHandle mh, final boolean isOptimistic) {
-        this(mh, null, isOptimistic);
+    public Specialization(final MethodHandle mh, final boolean isOptimistic, final boolean convertsNumericArgs) {
+        this(mh, null, isOptimistic, convertsNumericArgs);
     }
 
     /**
@@ -65,10 +67,13 @@
      *  if this can be linked on its first encounter, which is needed as per our standard linker semantics
      * @param isOptimistic is this an optimistic native method, i.e. can it throw {@link UnwarrantedOptimismException}
      *   which would have to lead to a relink and return value processing
+     * @param convertsNumericArgs true if it is safe to convert arguments to numbers
      */
-    public Specialization(final MethodHandle mh, final Class<? extends LinkLogic> linkLogicClass, final boolean isOptimistic) {
+    public Specialization(final MethodHandle mh, final Class<? extends LinkLogic> linkLogicClass,
+                          final boolean isOptimistic, final boolean convertsNumericArgs) {
         this.mh             = mh;
         this.isOptimistic   = isOptimistic;
+        this.convertsNumericArgs = convertsNumericArgs;
         if (linkLogicClass != null) {
             //null out the "empty" link logic class for optimization purposes
             //we only use the empty instance because we can't default class annotations
@@ -110,5 +115,15 @@
         return isOptimistic;
     }
 
+    /**
+     * Check if this function converts arguments for numeric parameters to numbers
+     * so it's safe to pass booleans as 0 and 1
+     *
+     * @return true if it is safe to convert arguments to numbers
+     */
+    public boolean convertsNumericArgs() {
+        return convertsNumericArgs;
+    }
+
 }
 
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/JDK-8170322.js	Fri Nov 25 14:20:24 2016 +0100
@@ -0,0 +1,42 @@
+/*
+ * Copyright (c) 2016, 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-8170322: Specialized functions convert booleans to numbers
+ *
+ * @test
+ * @run
+ */
+
+var array = [];
+array.push(true);
+array.push(false);
+
+Assert.assertTrue([].concat(true)[0] === true);
+Assert.assertTrue([].concat(false)[0] === false);
+
+Assert.assertTrue(array[0] === true);
+Assert.assertTrue(array[1] === false);
+
+Assert.assertTrue("foo".charAt(false) === 'f');
+Assert.assertTrue("foo".charAt(true) === 'o');