Mercurial > people > rkennke > jdk9-shenandoah-final > nashorn
changeset 833:899b6f171676
8038799: Guard and unbox boxed primitives types on setting them in Properties to avoid megamorphisism
Reviewed-by: attila, jlaskey
| author | lagergren |
|---|---|
| date | Tue, 01 Apr 2014 11:19:32 +0200 |
| parents | 7bb20a02bad0 |
| children | 1b9bd93570f8 |
| files | src/jdk/nashorn/internal/codegen/ObjectClassGenerator.java src/jdk/nashorn/internal/lookup/MethodHandleFactory.java src/jdk/nashorn/internal/objects/NativeObject.java src/jdk/nashorn/internal/runtime/AccessorProperty.java src/jdk/nashorn/internal/runtime/PropertyHashMap.java src/jdk/nashorn/internal/runtime/linker/LinkerCallSite.java test/script/basic/runsunspider-lazy.js.EXPECTED |
| diffstat | 7 files changed, 80 insertions(+), 15 deletions(-) [+] |
line wrap: on
line diff
--- a/src/jdk/nashorn/internal/codegen/ObjectClassGenerator.java Mon Mar 31 14:13:34 2014 +0200 +++ b/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/src/jdk/nashorn/internal/lookup/MethodHandleFactory.java Mon Mar 31 14:13:34 2014 +0200 +++ b/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/src/jdk/nashorn/internal/objects/NativeObject.java Mon Mar 31 14:13:34 2014 +0200 +++ b/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/src/jdk/nashorn/internal/runtime/AccessorProperty.java Mon Mar 31 14:13:34 2014 +0200 +++ b/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/src/jdk/nashorn/internal/runtime/PropertyHashMap.java Mon Mar 31 14:13:34 2014 +0200 +++ b/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(" -> ");