8137333: Boundless soft caching of property map histories causes high memory pressure
authorattila
Wed, 30 Sep 2015 10:09:44 +0200
changeset 32890 0118bc9769e1
parent 32889 aef39bbfac15
child 32891 687132a414b2
8137333: Boundless soft caching of property map histories causes high memory pressure Reviewed-by: hannesw, sundar
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/PropertyMap.java
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/PropertyMap.java	Mon Sep 28 18:58:52 2015 +0530
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/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<Object>, 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<String, SwitchPoint> protoSwitches;
 
     /** History of maps, used to limit map duplication. */
-    private transient WeakHashMap<Property, SoftReference<PropertyMap>> history;
+    private transient WeakHashMap<Property, Reference<PropertyMap>> history;
 
     /** History of prototypes, used to limit map duplication. */
     private transient WeakHashMap<ScriptObject, SoftReference<PropertyMap>> 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<PropertyMap> ref = history.get(property);
+            final Reference<PropertyMap> ref = history.get(property);
             final PropertyMap historicMap = ref == null ? null : ref.get();
 
             if (historicMap != null) {