# HG changeset patch # User lagergren # Date 1396343972 -7200 # Node ID da070553a8e1c2b5cf0e8d5fb22dbe8c4e16c8a4 # Parent 1e825be55fd16ea5e0695ee59cb11287e0f7050d 8038799: Guard and unbox boxed primitives types on setting them in Properties to avoid megamorphisism Reviewed-by: attila, jlaskey diff -r 1e825be55fd1 -r da070553a8e1 nashorn/src/jdk/nashorn/internal/codegen/ObjectClassGenerator.java --- 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 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 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 diff -r 1e825be55fd1 -r da070553a8e1 nashorn/src/jdk/nashorn/internal/lookup/MethodHandleFactory.java --- 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; } diff -r 1e825be55fd1 -r da070553a8e1 nashorn/src/jdk/nashorn/internal/objects/NativeObject.java --- 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 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); } diff -r 1e825be55fd1 -r da070553a8e1 nashorn/src/jdk/nashorn/internal/runtime/AccessorProperty.java --- 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); } diff -r 1e825be55fd1 -r da070553a8e1 nashorn/src/jdk/nashorn/internal/runtime/PropertyHashMap.java --- 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(" -> "); diff -r 1e825be55fd1 -r da070553a8e1 nashorn/src/jdk/nashorn/internal/runtime/linker/LinkerCallSite.java --- 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; } /** diff -r 1e825be55fd1 -r da070553a8e1 nashorn/test/script/basic/runsunspider-lazy.js.EXPECTED --- 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!