8038799: Guard and unbox boxed primitives types on setting them in Properties to avoid megamorphisism
authorlagergren
Tue, 01 Apr 2014 11:19:32 +0200
changeset 24734 da070553a8e1
parent 24733 1e825be55fd1
child 24735 9833d3ceed5b
8038799: Guard and unbox boxed primitives types on setting them in Properties to avoid megamorphisism Reviewed-by: attila, jlaskey
nashorn/src/jdk/nashorn/internal/codegen/ObjectClassGenerator.java
nashorn/src/jdk/nashorn/internal/lookup/MethodHandleFactory.java
nashorn/src/jdk/nashorn/internal/objects/NativeObject.java
nashorn/src/jdk/nashorn/internal/runtime/AccessorProperty.java
nashorn/src/jdk/nashorn/internal/runtime/PropertyHashMap.java
nashorn/src/jdk/nashorn/internal/runtime/linker/LinkerCallSite.java
nashorn/test/script/basic/runsunspider-lazy.js.EXPECTED
--- a/nashorn/src/jdk/nashorn/internal/codegen/ObjectClassGenerator.java	Mon Mar 31 14:13:34 2014 +0200
+++ b/nashorn/src/jdk/nashorn/internal/codegen/ObjectClassGenerator.java	Tue Apr 01 11:19:32 2014 +0200
@@ -76,7 +76,14 @@
 public final class ObjectClassGenerator {
 
     /**
-     * Marker for scope parameters.g
+     * Type guard to make sure we don't unnecessarily explode field storages. Rather unbox e.g.
+     * a java.lang.Number than blow up the field. Gradually, optimistic types should create almost
+     * no boxed types
+     */
+    private static final MethodHandle IS_TYPE_GUARD = findOwnMH("isType", boolean.class, Class.class, Object.class);
+
+    /**
+     * Marker for scope parameters
      */
     private static final String SCOPE_MARKER = "P";
 
@@ -778,6 +785,57 @@
         }
     }
 
+    @SuppressWarnings("unused")
+    private static boolean isType(final Class<?> boxedForType, final Object x) {
+        return x != null && x.getClass() == boxedForType;
+    }
+
+    private static Class<? extends Number> getBoxedType(final Class<?> forType) {
+        if (forType == int.class) {
+            return Integer.class;
+        }
+
+        if (forType == long.class) {
+            return Long.class;
+        }
+
+        if (forType == double.class) {
+            return Double.class;
+        }
+
+        assert false;
+        return null;
+    }
+
+    /**
+     * If we are setting boxed types (because the compiler couldn't determine which they were) to
+     * a primitive field, we can reuse the primitive field getter, as long as we are setting an element
+     * of the same boxed type as the primitive type representation
+     *
+     * @param forType           the current type
+     * @param primitiveSetter   primitive setter for the current type with an element of the current type
+     * @param objectSetter      the object setter
+     *
+     * @return method handle that checks if the element to be set is of the currenttype, even though it's boxed
+     *  and instead of using the generic object setter, that would blow up the type and invalidate the map,
+     *  unbox it and call the primitive setter instead
+     */
+    public static MethodHandle createGuardBoxedPrimitiveSetter(final Class<?> forType, final MethodHandle primitiveSetter, final MethodHandle objectSetter) {
+        final Class<? extends Number> boxedForType = getBoxedType(forType);
+        //object setter that checks for primitive if current type is primitive
+        return MH.guardWithTest(
+            MH.insertArguments(
+                MH.dropArguments(
+                    IS_TYPE_GUARD,
+                    1,
+                    Object.class),
+                0,
+                boxedForType),
+                MH.asType(
+                    primitiveSetter,
+                    objectSetter.type()),
+                objectSetter);
+    }
     /**
      * Add padding to field count to avoid creating too many classes and have some spare fields
      * @param count the field count
--- a/nashorn/src/jdk/nashorn/internal/lookup/MethodHandleFactory.java	Mon Mar 31 14:13:34 2014 +0200
+++ b/nashorn/src/jdk/nashorn/internal/lookup/MethodHandleFactory.java	Tue Apr 01 11:19:32 2014 +0200
@@ -142,7 +142,7 @@
         final String str = "    return" +
                 (VOID_TAG.equals(value) ?
                     ";" :
-                    " " + stripName(value) + "; // [type=" + (value == null ? "null" : stripName(value.getClass()) + ']'));
+                    " " + stripName(value) + "; // [type=" + (value == null ? "null]" : stripName(value.getClass()) + ']'));
         logger.log(TRACE_LEVEL, str);
         return value;
     }
--- a/nashorn/src/jdk/nashorn/internal/objects/NativeObject.java	Mon Mar 31 14:13:34 2014 +0200
+++ b/nashorn/src/jdk/nashorn/internal/objects/NativeObject.java	Tue Apr 01 11:19:32 2014 +0200
@@ -639,8 +639,9 @@
             final ArrayList<Property> propList = new ArrayList<>();
             for (final Property prop : properties) {
                 if (prop.isEnumerable()) {
-                    prop.setValue(sourceObj, sourceObj, sourceObj.get(prop.getKey()), false);
+                    final Object value = sourceObj.get(prop.getKey());
                     prop.setCurrentType(Object.class);
+                    prop.setValue(sourceObj, sourceObj, value, false);
                     propList.add(prop);
                 }
             }
@@ -739,7 +740,7 @@
         targetObj.addBoundProperties(source, properties.toArray(new AccessorProperty[properties.size()]));
     }
 
-    private static MethodHandle getBoundBeanMethodGetter(Object source, MethodHandle methodGetter) {
+    private static MethodHandle getBoundBeanMethodGetter(final Object source, final MethodHandle methodGetter) {
         try {
             // NOTE: we're relying on the fact that "dyn:getMethod:..." return value is constant for any given method
             // name and object linked with BeansLinker. (Actually, an even stronger assumption is true: return value is
@@ -773,7 +774,7 @@
         return guard == null || (boolean)guard.invoke(obj);
     }
 
-    private static LinkRequest createLinkRequest(String operation, MethodType methodType, Object source) {
+    private static LinkRequest createLinkRequest(final String operation, final MethodType methodType, final Object source) {
         return new LinkRequestImpl(CallSiteDescriptorFactory.create(MethodHandles.publicLookup(), operation,
                 methodType), null, 0, false, source);
     }
--- a/nashorn/src/jdk/nashorn/internal/runtime/AccessorProperty.java	Mon Mar 31 14:13:34 2014 +0200
+++ b/nashorn/src/jdk/nashorn/internal/runtime/AccessorProperty.java	Tue Apr 01 11:19:32 2014 +0200
@@ -42,6 +42,7 @@
 import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodHandles;
 import java.util.logging.Level;
+
 import jdk.nashorn.internal.codegen.ObjectClassGenerator;
 import jdk.nashorn.internal.codegen.types.Type;
 import jdk.nashorn.internal.lookup.Lookup;
@@ -168,6 +169,7 @@
         this.objectGetter    = bindTo(property.objectGetter, delegate);
         this.objectSetter    = bindTo(property.objectSetter, delegate);
 
+        property.GETTER_CACHE = new MethodHandle[NOOF_TYPES];
         // Properties created this way are bound to a delegate
         setCurrentType(property.getCurrentType());
     }
@@ -199,7 +201,6 @@
         this.primitiveSetter = primitiveSetter;
         this.objectGetter    = objectGetter;
         this.objectSetter    = objectSetter;
-//        setCurrentType(OBJECT_FIELDS_ONLY ? Object.class : initialType);
         initializeType();
     }
 
@@ -614,7 +615,6 @@
         return getCurrentType() == null;
     }
 
-    //TODO final
     @Override
     public MethodHandle getSetter(final Class<?> type, final PropertyMap currentMap) {
         final int      i       = getAccessorTypeIndex(type);
@@ -623,12 +623,16 @@
 
         //if we are asking for an object setter, but are still a primitive type, we might try to box it
         MethodHandle mh;
-
         if (needsInvalidator(i, ci)) {
             final Property     newProperty = getWiderProperty(type);
             final PropertyMap  newMap      = getWiderMap(currentMap, newProperty);
+
             final MethodHandle widerSetter = newProperty.getSetter(type, newMap);
-            mh = MH.filterArguments(widerSetter, 0, MH.insertArguments(REPLACE_MAP, 1, newMap, getKey(), getCurrentType(), type));
+            final Class<?>   ct = getCurrentType();
+            mh = MH.filterArguments(widerSetter, 0, MH.insertArguments(REPLACE_MAP, 1, newMap, getKey(), ct, type));
+            if (ct != null && ct.isPrimitive() && !type.isPrimitive()) {
+                 mh = ObjectClassGenerator.createGuardBoxedPrimitiveSetter(ct, generateSetter(ct, ct), mh);
+            }
         } else {
             mh = generateSetter(forType, type);
         }
--- a/nashorn/src/jdk/nashorn/internal/runtime/PropertyHashMap.java	Mon Mar 31 14:13:34 2014 +0200
+++ b/nashorn/src/jdk/nashorn/internal/runtime/PropertyHashMap.java	Tue Apr 01 11:19:32 2014 +0200
@@ -489,13 +489,16 @@
             return new Element(list.getLink(), property);
         }
 
-        Element previous = list;
+        final Element head = new Element(null, list.getProperty());
+        Element previous = head;
         for (Element element = list.getLink(); element != null; element = element.getLink()) {
             if (element.match(key, hashCode)) {
                 previous.setLink(new Element(element.getLink(), property));
-                return list;
+                return head;
             }
-            previous = element;
+            final Element next = new Element(null, element.getProperty());
+            previous.setLink(next);
+            previous = next;
         }
         return list;
     }
@@ -680,7 +683,7 @@
 
             Element elem = this;
             do {
-                sb.append(elem.getValue().toStringShort());
+                sb.append(elem.getValue());
                 elem = elem.link;
                 if (elem != null) {
                     sb.append(" -> ");
--- a/nashorn/src/jdk/nashorn/internal/runtime/linker/LinkerCallSite.java	Mon Mar 31 14:13:34 2014 +0200
+++ b/nashorn/src/jdk/nashorn/internal/runtime/linker/LinkerCallSite.java	Tue Apr 01 11:19:32 2014 +0200
@@ -519,7 +519,7 @@
 
     @Override
     protected int getMaxChainLength() {
-        return 16;
+        return 8;
     }
 
     /**
--- a/nashorn/test/script/basic/runsunspider-lazy.js.EXPECTED	Mon Mar 31 14:13:34 2014 +0200
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,1 +0,0 @@
-Sunspider finished!