8081062: ListAdapter should take advantage of JSObject
authorattila
Tue, 26 May 2015 16:12:23 +0200
changeset 30832 9553298028ef
parent 30831 f733f1de0166
child 30833 7ffaac79bd34
8081062: ListAdapter should take advantage of JSObject Reviewed-by: lagergren, sundar
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/JSObjectListAdapter.java
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/ListAdapter.java
nashorn/test/src/jdk/nashorn/internal/runtime/test/JDK_8081015_Test.java
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/JSObjectListAdapter.java	Tue May 26 14:37:14 2015 +0200
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,56 +0,0 @@
-/*
- * Copyright (c) 2010, 2013, 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.internal.runtime;
-
-import jdk.nashorn.api.scripting.JSObject;
-
-/**
- * A ListAdapter that can wraps a JSObject.
- */
-public final class JSObjectListAdapter extends ListAdapter {
-    /**
-     * Creates a new list wrapper for the specified JSObject.
-     * @param obj JSOcript the object to wrap
-     */
-    public JSObjectListAdapter(final JSObject obj) {
-        super(obj);
-    }
-
-    @Override
-    public int size() {
-        return JSType.toInt32(((JSObject)obj).getMember("length"));
-    }
-
-    @Override
-    protected Object getAt(final int index) {
-        return ((JSObject)obj).getSlot(index);
-    }
-
-    @Override
-    protected void setAt(final int index, final Object element) {
-        ((JSObject)obj).setSlot(index, element);
-    }
-}
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/ListAdapter.java	Tue May 26 14:37:14 2015 +0200
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/ListAdapter.java	Tue May 26 16:12:23 2015 +0200
@@ -25,17 +25,18 @@
 
 package jdk.nashorn.internal.runtime;
 
+import java.lang.invoke.MethodHandle;
 import java.util.AbstractList;
 import java.util.Deque;
 import java.util.Iterator;
 import java.util.ListIterator;
 import java.util.NoSuchElementException;
+import java.util.Objects;
 import java.util.RandomAccess;
 import java.util.concurrent.Callable;
 import jdk.nashorn.api.scripting.JSObject;
 import jdk.nashorn.api.scripting.ScriptObjectMirror;
 import jdk.nashorn.internal.runtime.linker.Bootstrap;
-import jdk.nashorn.internal.runtime.linker.InvokeByName;
 
 /**
  * An adapter that can wrap any ECMAScript Array-like object (that adheres to the array rules for the property
@@ -50,81 +51,56 @@
  * operations respectively, while {@link #addLast(Object)} and {@link #removeLast()} will translate to {@code push} and
  * {@code pop}.
  */
-public abstract class ListAdapter extends AbstractList<Object> implements RandomAccess, Deque<Object> {
-    // These add to the back and front of the list
-    private static final Object PUSH    = new Object();
-    private static InvokeByName getPUSH() {
-        return Context.getGlobal().getInvokeByName(PUSH,
-                new Callable<InvokeByName>() {
-                    @Override
-                    public InvokeByName call() {
-                        return new InvokeByName("push", Object.class, void.class, Object.class);
-                    }
-                });
+public final class ListAdapter extends AbstractList<Object> implements RandomAccess, Deque<Object> {
+    // Invoker creator for methods that add to the start or end of the list: PUSH and UNSHIFT. Takes fn, this, and value, returns void.
+    private static final Callable<MethodHandle> ADD_INVOKER_CREATOR = invokerCreator(void.class, Object.class, JSObject.class, Object.class);
+
+    // PUSH adds to the end of the list
+    private static final Object PUSH = new Object();
+    private static MethodHandle getPushInvoker() {
+        return getDynamicInvoker(PUSH, ADD_INVOKER_CREATOR);
     }
 
+    // UNSHIFT adds to the start of the list
     private static final Object UNSHIFT = new Object();
-    private static InvokeByName getUNSHIFT() {
-        return Context.getGlobal().getInvokeByName(UNSHIFT,
-                new Callable<InvokeByName>() {
-                    @Override
-                    public InvokeByName call() {
-                        return new InvokeByName("unshift", Object.class, void.class, Object.class);
-                    }
-                });
+    private static MethodHandle getUnshiftInvoker() {
+        return getDynamicInvoker(UNSHIFT, ADD_INVOKER_CREATOR);
     }
 
-    // These remove from the back and front of the list
+    // Invoker creator for methods that remove from the tail or head of the list: POP and SHIFT. Takes fn, this, returns Object.
+    private static final Callable<MethodHandle> REMOVE_INVOKER_CREATOR = invokerCreator(Object.class, Object.class, JSObject.class);
+
+    // POP removes from the to the end of the list
     private static final Object POP = new Object();
-    private static InvokeByName getPOP() {
-        return Context.getGlobal().getInvokeByName(POP,
-                new Callable<InvokeByName>() {
-                    @Override
-                    public InvokeByName call() {
-                        return new InvokeByName("pop", Object.class, Object.class);
-                    }
-                });
+    private static MethodHandle getPopInvoker() {
+        return getDynamicInvoker(POP, REMOVE_INVOKER_CREATOR);
     }
 
+    // SHIFT removes from the to the start of the list
     private static final Object SHIFT = new Object();
-    private static InvokeByName getSHIFT() {
-        return Context.getGlobal().getInvokeByName(SHIFT,
-                new Callable<InvokeByName>() {
-                    @Override
-                    public InvokeByName call() {
-                        return new InvokeByName("shift", Object.class, Object.class);
-                    }
-                });
+    private static MethodHandle getShiftInvoker() {
+        return getDynamicInvoker(SHIFT, REMOVE_INVOKER_CREATOR);
     }
 
-    // These insert and remove in the middle of the list
+    // SPLICE can be used to add a value in the middle of the list.
     private static final Object SPLICE_ADD = new Object();
-    private static InvokeByName getSPLICE_ADD() {
-        return Context.getGlobal().getInvokeByName(SPLICE_ADD,
-                new Callable<InvokeByName>() {
-                    @Override
-                    public InvokeByName call() {
-                        return new InvokeByName("splice", Object.class, void.class, int.class, int.class, Object.class);
-                    }
-                });
+    private static final Callable<MethodHandle> SPLICE_ADD_INVOKER_CREATOR = invokerCreator(void.class, Object.class, JSObject.class, int.class, int.class, Object.class);
+    private static MethodHandle getSpliceAddInvoker() {
+        return getDynamicInvoker(SPLICE_ADD, SPLICE_ADD_INVOKER_CREATOR);
     }
 
+    // SPLICE can also be used to remove values from the middle of the list.
     private static final Object SPLICE_REMOVE = new Object();
-    private static InvokeByName getSPLICE_REMOVE() {
-        return  Context.getGlobal().getInvokeByName(SPLICE_REMOVE,
-                new Callable<InvokeByName>() {
-                    @Override
-                    public InvokeByName call() {
-                        return new InvokeByName("splice", Object.class, void.class, int.class, int.class);
-                    }
-                });
+    private static final Callable<MethodHandle> SPLICE_REMOVE_INVOKER_CREATOR = invokerCreator(void.class, Object.class, JSObject.class, int.class, int.class);
+    private static MethodHandle getSpliceRemoveInvoker() {
+        return getDynamicInvoker(SPLICE_REMOVE, SPLICE_REMOVE_INVOKER_CREATOR);
     }
 
     /** wrapped object */
-    protected final Object obj;
+    protected final JSObject obj;
 
     // allow subclasses only in this package
-    ListAdapter(final Object obj) {
+    ListAdapter(final JSObject obj) {
         this.obj = obj;
     }
 
@@ -135,14 +111,16 @@
      * @return A ListAdapter wrapper object
      */
     public static ListAdapter create(final Object obj) {
+        return new ListAdapter(getJSObject(obj));
+    }
+
+    private static JSObject getJSObject(final Object obj) {
         if (obj instanceof ScriptObject) {
-            final Object mirror = ScriptObjectMirror.wrap(obj, Context.getGlobal());
-            return new JSObjectListAdapter((JSObject)mirror);
+            return (JSObject)ScriptObjectMirror.wrap(obj, Context.getGlobal());
         } else if (obj instanceof JSObject) {
-            return new JSObjectListAdapter((JSObject)obj);
-        } else {
-            throw new IllegalArgumentException("ScriptObject or JSObject expected");
+            return (JSObject)obj;
         }
+        throw new IllegalArgumentException("ScriptObject or JSObject expected");
     }
 
     @Override
@@ -151,28 +129,18 @@
         return getAt(index);
     }
 
-    /**
-     * Get object at an index
-     * @param index index in list
-     * @return object
-     */
-    protected abstract Object getAt(final int index);
+    private Object getAt(final int index) {
+        return obj.getSlot(index);
+    }
 
     @Override
     public Object set(final int index, final Object element) {
         checkRange(index);
         final Object prevValue = getAt(index);
-        setAt(index, element);
+        obj.setSlot(index, element);
         return prevValue;
     }
 
-    /**
-     * Set object at an index
-     * @param index   index in list
-     * @param element element
-     */
-    protected abstract void setAt(final int index, final Object element);
-
     private void checkRange(final int index) {
         if(index < 0 || index >= size()) {
             throw invalidIndex(index);
@@ -180,6 +148,11 @@
     }
 
     @Override
+    public int size() {
+        return JSType.toInt32(obj.getMember("length"));
+    }
+
+    @Override
     public final void push(final Object e) {
         addFirst(e);
     }
@@ -193,10 +166,7 @@
     @Override
     public final void addFirst(final Object e) {
         try {
-            final InvokeByName unshiftInvoker = getUNSHIFT();
-            final Object fn = unshiftInvoker.getGetter().invokeExact(obj);
-            checkFunction(fn, unshiftInvoker);
-            unshiftInvoker.getInvoker().invokeExact(fn, obj, e);
+            getUnshiftInvoker().invokeExact(getFunction("unshift"), obj, e);
         } catch(RuntimeException | Error ex) {
             throw ex;
         } catch(final Throwable t) {
@@ -207,10 +177,7 @@
     @Override
     public final void addLast(final Object e) {
         try {
-            final InvokeByName pushInvoker = getPUSH();
-            final Object fn = pushInvoker.getGetter().invokeExact(obj);
-            checkFunction(fn, pushInvoker);
-            pushInvoker.getInvoker().invokeExact(fn, obj, e);
+            getPushInvoker().invokeExact(getFunction("push"), obj, e);
         } catch(RuntimeException | Error ex) {
             throw ex;
         } catch(final Throwable t) {
@@ -228,10 +195,7 @@
             } else {
                 final int size = size();
                 if(index < size) {
-                    final InvokeByName spliceAddInvoker = getSPLICE_ADD();
-                    final Object fn = spliceAddInvoker.getGetter().invokeExact(obj);
-                    checkFunction(fn, spliceAddInvoker);
-                    spliceAddInvoker.getInvoker().invokeExact(fn, obj, index, 0, e);
+                    getSpliceAddInvoker().invokeExact(obj.getMember("splice"), obj, index, 0, e);
                 } else if(index == size) {
                     addLast(e);
                 } else {
@@ -244,10 +208,12 @@
             throw new RuntimeException(t);
         }
     }
-    private static void checkFunction(final Object fn, final InvokeByName invoke) {
+    private Object getFunction(final String name) {
+        final Object fn = obj.getMember(name);
         if(!(Bootstrap.isCallable(fn))) {
-            throw new UnsupportedOperationException("The script object doesn't have a function named " + invoke.getName());
+            throw new UnsupportedOperationException("The script object doesn't have a function named " + name);
         }
+        return fn;
     }
 
     private static IndexOutOfBoundsException invalidIndex(final int index) {
@@ -321,10 +287,7 @@
 
     private Object invokeShift() {
         try {
-            final InvokeByName shiftInvoker = getSHIFT();
-            final Object fn = shiftInvoker.getGetter().invokeExact(obj);
-            checkFunction(fn, shiftInvoker);
-            return shiftInvoker.getInvoker().invokeExact(fn, obj);
+            return getShiftInvoker().invokeExact(getFunction("shift"), obj);
         } catch(RuntimeException | Error ex) {
             throw ex;
         } catch(final Throwable t) {
@@ -334,10 +297,7 @@
 
     private Object invokePop() {
         try {
-            final InvokeByName popInvoker = getPOP();
-            final Object fn = popInvoker.getGetter().invokeExact(obj);
-            checkFunction(fn, popInvoker);
-            return popInvoker.getInvoker().invokeExact(fn, obj);
+            return getPopInvoker().invokeExact(getFunction("pop"), obj);
         } catch(RuntimeException | Error ex) {
             throw ex;
         } catch(final Throwable t) {
@@ -352,10 +312,7 @@
 
     private void invokeSpliceRemove(final int fromIndex, final int count) {
         try {
-            final InvokeByName spliceRemoveInvoker = getSPLICE_REMOVE();
-            final Object fn = spliceRemoveInvoker.getGetter().invokeExact(obj);
-            checkFunction(fn, spliceRemoveInvoker);
-            spliceRemoveInvoker.getInvoker().invokeExact(fn, obj, fromIndex, count);
+            getSpliceRemoveInvoker().invokeExact(getFunction("splice"), obj, fromIndex, count);
         } catch(RuntimeException | Error ex) {
             throw ex;
         } catch(final Throwable t) {
@@ -443,12 +400,24 @@
 
     private static boolean removeOccurrence(final Object o, final Iterator<Object> it) {
         while(it.hasNext()) {
-            final Object e = it.next();
-            if(o == null ? e == null : o.equals(e)) {
+            if(Objects.equals(o, it.next())) {
                 it.remove();
                 return true;
             }
         }
         return false;
     }
+
+    private static Callable<MethodHandle> invokerCreator(final Class<?> rtype, final Class<?>... ptypes) {
+        return new Callable<MethodHandle>() {
+            @Override
+            public MethodHandle call() {
+                return Bootstrap.createDynamicInvoker("dyn:call", rtype, ptypes);
+            }
+        };
+    }
+
+    private static MethodHandle getDynamicInvoker(final Object key, final Callable<MethodHandle> creator) {
+        return Context.getGlobal().getDynamicInvoker(key, creator);
+    }
 }
--- a/nashorn/test/src/jdk/nashorn/internal/runtime/test/JDK_8081015_Test.java	Tue May 26 14:37:14 2015 +0200
+++ b/nashorn/test/src/jdk/nashorn/internal/runtime/test/JDK_8081015_Test.java	Tue May 26 16:12:23 2015 +0200
@@ -28,19 +28,19 @@
 import static org.testng.Assert.assertEquals;
 import static org.testng.Assert.assertNull;
 
-import jdk.nashorn.test.models.JDK_8081015_TestModel;
-
 import java.util.Collection;
 import java.util.Queue;
 import javax.script.ScriptEngine;
 import javax.script.ScriptException;
 import jdk.nashorn.api.scripting.NashornScriptEngineFactory;
+import jdk.nashorn.test.models.JDK_8081015_TestModel;
 import org.testng.annotations.Test;
 
 /**
  * @bug 8081015
  * @summary Test that native arrays get converted to {@link Queue} and {@link Collection}.
  */
+@SuppressWarnings("javadoc")
 public class JDK_8081015_Test {
     @Test
     public void testConvertToCollection() throws ScriptException {
@@ -62,7 +62,7 @@
         test("receiveQueue");
     }
 
-    private void test(final String methodName) throws ScriptException {
+    private static void test(final String methodName) throws ScriptException {
         final ScriptEngine engine = new NashornScriptEngineFactory().getScriptEngine();
         final JDK_8081015_TestModel model = new JDK_8081015_TestModel();
         engine.put("test", model);