8171849: Collection and Queue conversions not prioritized for Arrays
authorattila
Thu, 22 Dec 2016 18:13:41 +0100
changeset 42959 c2cff53239e0
parent 42958 4975a3a0dac4
child 42960 837a2f16bf3c
8171849: Collection and Queue conversions not prioritized for Arrays Reviewed-by: hannesw, jlaskey
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/linker/NashornLinker.java
nashorn/test/script/basic/JDK-8171849.js
nashorn/test/src/jdk/nashorn/test/models/ArrayConversionPreferences.java
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/linker/NashornLinker.java	Thu Dec 22 16:51:07 2016 +0100
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/linker/NashornLinker.java	Thu Dec 22 18:13:41 2016 +0100
@@ -285,15 +285,15 @@
     @Override
     public Comparison compareConversion(final Class<?> sourceType, final Class<?> targetType1, final Class<?> targetType2) {
         if(sourceType == NativeArray.class) {
-            // Prefer lists, as they're less costly to create than arrays.
-            if(isList(targetType1)) {
-                if(!isList(targetType2)) {
+            // Prefer those types we can convert to with just a wrapper (cheaper than Java array creation).
+            if(isArrayPreferredTarget(targetType1)) {
+                if(!isArrayPreferredTarget(targetType2)) {
                     return Comparison.TYPE_1_BETTER;
                 }
-            } else if(isList(targetType2)) {
+            } else if(isArrayPreferredTarget(targetType2)) {
                 return Comparison.TYPE_2_BETTER;
             }
-            // Then prefer arrays
+            // Then prefer Java arrays
             if(targetType1.isArray()) {
                 if(!targetType2.isArray()) {
                     return Comparison.TYPE_1_BETTER;
@@ -315,8 +315,8 @@
         return Comparison.INDETERMINATE;
     }
 
-    private static boolean isList(final Class<?> clazz) {
-        return clazz == List.class || clazz == Deque.class;
+    private static boolean isArrayPreferredTarget(final Class<?> clazz) {
+        return clazz == List.class || clazz == Collection.class || clazz == Queue.class || clazz == Deque.class;
     }
 
     private static final MethodHandle IS_SCRIPT_OBJECT = Guards.isInstance(ScriptObject.class, MH.type(Boolean.TYPE, Object.class));
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/JDK-8171849.js	Thu Dec 22 18:13:41 2016 +0100
@@ -0,0 +1,47 @@
+/*
+ * 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-8171849: Collection and Queue conversions not prioritized for Arrays
+ *
+ * @test
+ * @run
+ */
+
+var acp = new (Java.type("jdk.nashorn.test.models.ArrayConversionPreferences"))
+
+var a = [1, "", {}]
+
+Assert.assertTrue(acp.testCollectionOverMap(a))
+Assert.assertTrue(acp.testCollectionOverArray(a))
+Assert.assertTrue(acp.testListOverMap(a))
+Assert.assertTrue(acp.testListOverArray(a))
+Assert.assertTrue(acp.testListOverCollection(a))
+Assert.assertTrue(acp.testQueueOverMap(a))
+Assert.assertTrue(acp.testQueueOverArray(a))
+Assert.assertTrue(acp.testQueueOverCollection(a))
+Assert.assertTrue(acp.testDequeOverMap(a))
+Assert.assertTrue(acp.testDequeOverArray(a))
+Assert.assertTrue(acp.testDequeOverCollection(a))
+Assert.assertTrue(acp.testDequeOverQueue(a))
+Assert.assertTrue(acp.testArrayOverMap(a))
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/src/jdk/nashorn/test/models/ArrayConversionPreferences.java	Thu Dec 22 18:13:41 2016 +0100
@@ -0,0 +1,74 @@
+/*
+ * 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.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * 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.
+ */
+
+package jdk.nashorn.test.models;
+
+import java.util.Collection;
+import java.util.Deque;
+import java.util.List;
+import java.util.Map;
+import java.util.Queue;
+
+public class ArrayConversionPreferences {
+    public boolean testCollectionOverMap(final Collection x) { return true; }
+    public boolean testCollectionOverMap(final Map x) { return false; }
+
+    public boolean testCollectionOverArray(final Collection x) { return true; }
+    public boolean testCollectionOverArray(final Object[] x) { return false; }
+
+    public boolean testListOverMap(final List x) { return true; }
+    public boolean testListOverMap(final Map x) { return false; }
+
+    public boolean testListOverArray(final List x) { return true; }
+    public boolean testListOverArray(final Object[] x) { return false; }
+
+    public boolean testListOverCollection(final List x) { return true; }
+    public boolean testListOverCollection(final Collection x) { return false; }
+
+    public boolean testQueueOverMap(final Queue x) { return true; }
+    public boolean testQueueOverMap(final Map x) { return false; }
+
+    public boolean testQueueOverArray(final Queue x) { return true; }
+    public boolean testQueueOverArray(final Object[] x) { return false; }
+
+    public boolean testQueueOverCollection(final Queue x) { return true; }
+    public boolean testQueueOverCollection(final Collection x) { return false; }
+
+    public boolean testDequeOverMap(final Deque x) { return true; }
+    public boolean testDequeOverMap(final Map x) { return false; }
+
+    public boolean testDequeOverArray(final Deque x) { return true; }
+    public boolean testDequeOverArray(final Object[] x) { return false; }
+
+    public boolean testDequeOverCollection(final Deque x) { return true; }
+    public boolean testDequeOverCollection(final Collection x) { return false; }
+
+    public boolean testDequeOverQueue(final Deque x) { return true; }
+    public boolean testDequeOverQueue(final Queue x) { return false; }
+
+    public boolean testArrayOverMap(final Object[] x) { return true; }
+    public boolean testArrayOverMap(final Map x) { return false; }
+}
+