# HG changeset patch # User attila # Date 1443600584 -7200 # Node ID c2147c431c293a8044dd757bb71081cf1844fb3a # Parent dfa57c580b6a1f74a0aeea1a55524cd4470ae009 8137333: Boundless soft caching of property map histories causes high memory pressure Reviewed-by: hannesw, sundar diff -r dfa57c580b6a -r c2147c431c29 src/jdk/nashorn/internal/runtime/PropertyMap.java --- a/src/jdk/nashorn/internal/runtime/PropertyMap.java Mon Sep 28 18:58:52 2015 +0530 +++ b/src/jdk/nashorn/internal/runtime/PropertyMap.java Wed Sep 30 10:09:44 2015 +0200 @@ -34,7 +34,9 @@ import java.io.ObjectOutputStream; import java.io.Serializable; import java.lang.invoke.SwitchPoint; +import java.lang.ref.Reference; import java.lang.ref.SoftReference; +import java.lang.ref.WeakReference; import java.util.Arrays; import java.util.BitSet; import java.util.Collection; @@ -43,6 +45,7 @@ import java.util.NoSuchElementException; import java.util.WeakHashMap; import java.util.concurrent.atomic.LongAdder; +import jdk.nashorn.internal.runtime.options.Options; import jdk.nashorn.internal.scripts.JO; /** @@ -55,6 +58,9 @@ * will return a new map. */ public class PropertyMap implements Iterable, Serializable { + private static final int INITIAL_SOFT_REFERENCE_DERIVATION_LIMIT = + Math.max(0, Options.getIntProperty("nashorn.propertyMap.softReferenceDerivationLimit", 32)); + /** Used for non extensible PropertyMaps, negative logic as the normal case is extensible. See {@link ScriptObject#preventExtensions()} */ private static final int NOT_EXTENSIBLE = 0b0000_0001; /** Does this map contain valid array keys? */ @@ -78,6 +84,13 @@ /** Structure class name */ private final String className; + /** + * Countdown of number of times this property map has been derived from another property map. When it + * reaches zero, the property map will start using weak references instead of soft references to hold on + * to its history elements. + */ + private final int softReferenceDerivationLimit; + /** A reference to the expected shared prototype property map. If this is set this * property map should only be used if it the same as the actual prototype map. */ private transient SharedPropertyMap sharedProtoMap; @@ -86,7 +99,7 @@ private transient HashMap protoSwitches; /** History of maps, used to limit map duplication. */ - private transient WeakHashMap> history; + private transient WeakHashMap> history; /** History of prototypes, used to limit map duplication. */ private transient WeakHashMap> protoHistory; @@ -114,6 +127,7 @@ this.fieldMaximum = fieldMaximum; this.spillLength = spillLength; this.flags = flags; + this.softReferenceDerivationLimit = INITIAL_SOFT_REFERENCE_DERIVATION_LIMIT; if (Context.DEBUG) { count.increment(); @@ -126,7 +140,7 @@ * @param propertyMap Existing property map. * @param properties A {@link PropertyHashMap} with a new set of properties. */ - private PropertyMap(final PropertyMap propertyMap, final PropertyHashMap properties, final int flags, final int fieldCount, final int spillLength) { + private PropertyMap(final PropertyMap propertyMap, final PropertyHashMap properties, final int flags, final int fieldCount, final int spillLength, final int softReferenceDerivationLimit) { this.properties = properties; this.flags = flags; this.spillLength = spillLength; @@ -137,6 +151,7 @@ this.listeners = propertyMap.listeners; this.freeSlots = propertyMap.freeSlots; this.sharedProtoMap = propertyMap.sharedProtoMap; + this.softReferenceDerivationLimit = softReferenceDerivationLimit; if (Context.DEBUG) { count.increment(); @@ -150,7 +165,7 @@ * @param propertyMap Existing property map. */ protected PropertyMap(final PropertyMap propertyMap) { - this(propertyMap, propertyMap.properties, propertyMap.flags, propertyMap.fieldCount, propertyMap.spillLength); + this(propertyMap, propertyMap.properties, propertyMap.flags, propertyMap.fieldCount, propertyMap.spillLength, propertyMap.softReferenceDerivationLimit); } private void writeObject(final ObjectOutputStream out) throws IOException { @@ -438,11 +453,7 @@ */ public final PropertyMap addPropertyNoHistory(final Property property) { propertyAdded(property, true); - final PropertyHashMap newProperties = properties.immutableAdd(property); - final PropertyMap newMap = new PropertyMap(this, newProperties, newFlags(property), newFieldCount(property), newSpillLength(property)); - newMap.updateFreeSlots(null, property); - - return newMap; + return addPropertyInternal(property); } /** @@ -457,15 +468,24 @@ PropertyMap newMap = checkHistory(property); if (newMap == null) { - final PropertyHashMap newProperties = properties.immutableAdd(property); - newMap = new PropertyMap(this, newProperties, newFlags(property), newFieldCount(property), newSpillLength(property)); - newMap.updateFreeSlots(null, property); + newMap = addPropertyInternal(property); addToHistory(property, newMap); } return newMap; } + private PropertyMap deriveMap(final PropertyHashMap newProperties, final int newFlags, final int newFieldCount, final int newSpillLength) { + return new PropertyMap(this, newProperties, newFlags, newFieldCount, newSpillLength, softReferenceDerivationLimit == 0 ? 0 : softReferenceDerivationLimit - 1); + } + + private PropertyMap addPropertyInternal(final Property property) { + final PropertyHashMap newProperties = properties.immutableAdd(property); + final PropertyMap newMap = deriveMap(newProperties, newFlags(property), newFieldCount(property), newSpillLength(property)); + newMap.updateFreeSlots(null, property); + return newMap; + } + /** * Remove a property from a map. Cloning or using an existing map if available. * @@ -485,13 +505,13 @@ // If deleted property was last field or spill slot we can make it reusable by reducing field/slot count. // Otherwise mark it as free in free slots bitset. if (isSpill && slot >= 0 && slot == spillLength - 1) { - newMap = new PropertyMap(this, newProperties, flags, fieldCount, spillLength - 1); + newMap = deriveMap(newProperties, flags, fieldCount, spillLength - 1); newMap.freeSlots = freeSlots; } else if (!isSpill && slot >= 0 && slot == fieldCount - 1) { - newMap = new PropertyMap(this, newProperties, flags, fieldCount - 1, spillLength); + newMap = deriveMap(newProperties, flags, fieldCount - 1, spillLength); newMap.freeSlots = freeSlots; } else { - newMap = new PropertyMap(this, newProperties, flags, fieldCount, spillLength); + newMap = deriveMap(newProperties, flags, fieldCount, spillLength); newMap.updateFreeSlots(property, null); } addToHistory(property, newMap); @@ -539,7 +559,7 @@ // Add replaces existing property. final PropertyHashMap newProperties = properties.immutableReplace(oldProperty, newProperty); - final PropertyMap newMap = new PropertyMap(this, newProperties, flags, fieldCount, newSpillLength); + final PropertyMap newMap = deriveMap(newProperties, flags, fieldCount, newSpillLength); if (!sameType) { newMap.updateFreeSlots(oldProperty, newProperty); @@ -584,7 +604,7 @@ final Property[] otherProperties = other.properties.getProperties(); final PropertyHashMap newProperties = properties.immutableAdd(otherProperties); - final PropertyMap newMap = new PropertyMap(this, newProperties, flags, fieldCount, spillLength); + final PropertyMap newMap = deriveMap(newProperties, flags, fieldCount, spillLength); for (final Property property : otherProperties) { // This method is only safe to use with non-slotted, native getter/setter properties assert property.getSlot() == -1; @@ -618,7 +638,7 @@ * @return New map with {@link #NOT_EXTENSIBLE} flag set. */ PropertyMap preventExtensions() { - return new PropertyMap(this, properties, flags | NOT_EXTENSIBLE, fieldCount, spillLength); + return deriveMap(properties, flags | NOT_EXTENSIBLE, fieldCount, spillLength); } /** @@ -634,7 +654,7 @@ newProperties = newProperties.immutableAdd(oldProperty.addFlags(Property.NOT_CONFIGURABLE)); } - return new PropertyMap(this, newProperties, flags | NOT_EXTENSIBLE, fieldCount, spillLength); + return deriveMap(newProperties, flags | NOT_EXTENSIBLE, fieldCount, spillLength); } /** @@ -656,7 +676,7 @@ newProperties = newProperties.immutableAdd(oldProperty.addFlags(propertyFlags)); } - return new PropertyMap(this, newProperties, flags | NOT_EXTENSIBLE, fieldCount, spillLength); + return deriveMap(newProperties, flags | NOT_EXTENSIBLE, fieldCount, spillLength); } /** @@ -743,7 +763,7 @@ history = new WeakHashMap<>(); } - history.put(property, new SoftReference<>(newMap)); + history.put(property, softReferenceDerivationLimit == 0 ? new WeakReference<>(newMap) : new SoftReference<>(newMap)); } /** @@ -756,7 +776,7 @@ private PropertyMap checkHistory(final Property property) { if (history != null) { - final SoftReference ref = history.get(property); + final Reference ref = history.get(property); final PropertyMap historicMap = ref == null ? null : ref.get(); if (historicMap != null) {