8038799: Guard and unbox boxed primitives types on setting them in Properties to avoid megamorphisism
Reviewed-by: attila, jlaskey
diff --git a/nashorn/src/jdk/nashorn/internal/codegen/ObjectClassGenerator.java b/nashorn/src/jdk/nashorn/internal/codegen/ObjectClassGenerator.java
index ca546c0..b97083e 100644
--- a/nashorn/src/jdk/nashorn/internal/codegen/ObjectClassGenerator.java
+++ b/nashorn/src/jdk/nashorn/internal/codegen/ObjectClassGenerator.java
@@ -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
diff --git a/nashorn/src/jdk/nashorn/internal/lookup/MethodHandleFactory.java b/nashorn/src/jdk/nashorn/internal/lookup/MethodHandleFactory.java
index b03e5c8..aa50874 100644
--- a/nashorn/src/jdk/nashorn/internal/lookup/MethodHandleFactory.java
+++ b/nashorn/src/jdk/nashorn/internal/lookup/MethodHandleFactory.java
@@ -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 --git a/nashorn/src/jdk/nashorn/internal/objects/NativeObject.java b/nashorn/src/jdk/nashorn/internal/objects/NativeObject.java
index 1cafd51..0e07217 100644
--- a/nashorn/src/jdk/nashorn/internal/objects/NativeObject.java
+++ b/nashorn/src/jdk/nashorn/internal/objects/NativeObject.java
@@ -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);
}
diff --git a/nashorn/src/jdk/nashorn/internal/runtime/AccessorProperty.java b/nashorn/src/jdk/nashorn/internal/runtime/AccessorProperty.java
index 269edbb..bbea21d 100644
--- a/nashorn/src/jdk/nashorn/internal/runtime/AccessorProperty.java
+++ b/nashorn/src/jdk/nashorn/internal/runtime/AccessorProperty.java
@@ -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 --git a/nashorn/src/jdk/nashorn/internal/runtime/PropertyHashMap.java b/nashorn/src/jdk/nashorn/internal/runtime/PropertyHashMap.java
index 877f2ad..8c20a23 100644
--- a/nashorn/src/jdk/nashorn/internal/runtime/PropertyHashMap.java
+++ b/nashorn/src/jdk/nashorn/internal/runtime/PropertyHashMap.java
@@ -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 --git a/nashorn/src/jdk/nashorn/internal/runtime/linker/LinkerCallSite.java b/nashorn/src/jdk/nashorn/internal/runtime/linker/LinkerCallSite.java
index 1634898..f7c7eb1 100644
--- a/nashorn/src/jdk/nashorn/internal/runtime/linker/LinkerCallSite.java
+++ b/nashorn/src/jdk/nashorn/internal/runtime/linker/LinkerCallSite.java
@@ -519,7 +519,7 @@
@Override
protected int getMaxChainLength() {
- return 16;
+ return 8;
}
/**
diff --git a/nashorn/test/script/basic/runsunspider-lazy.js.EXPECTED b/nashorn/test/script/basic/runsunspider-lazy.js.EXPECTED
deleted file mode 100644
index dd36083..0000000
--- a/nashorn/test/script/basic/runsunspider-lazy.js.EXPECTED
+++ /dev/null
@@ -1 +0,0 @@
-Sunspider finished!
