8006559: Octane:pdfjs leaks memory, runs slower iteration to iteration
authorhannesw
Fri, 26 Apr 2013 17:35:40 +0200
changeset 17252 9aeb443c4740
parent 17251 444648345fa6
child 17253 e374dcf3661b
8006559: Octane:pdfjs leaks memory, runs slower iteration to iteration Reviewed-by: attila, sundar, jlaskey
nashorn/buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/ConstructorGenerator.java
nashorn/buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/StringConstants.java
nashorn/src/jdk/nashorn/internal/objects/BoundScriptFunctionImpl.java
nashorn/src/jdk/nashorn/internal/objects/NativeDebug.java
nashorn/src/jdk/nashorn/internal/objects/ScriptFunctionImpl.java
nashorn/src/jdk/nashorn/internal/runtime/PropertyListenerManager.java
nashorn/src/jdk/nashorn/internal/runtime/ScriptFunction.java
--- a/nashorn/buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/ConstructorGenerator.java	Fri Apr 26 18:31:42 2013 +0530
+++ b/nashorn/buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/ConstructorGenerator.java	Fri Apr 26 17:35:40 2013 +0200
@@ -38,7 +38,6 @@
 import static jdk.nashorn.internal.tools.nasgen.StringConstants.MAP_FIELD_NAME;
 import static jdk.nashorn.internal.tools.nasgen.StringConstants.MAP_TYPE;
 import static jdk.nashorn.internal.tools.nasgen.StringConstants.OBJECT_DESC;
-import static jdk.nashorn.internal.tools.nasgen.StringConstants.PROTOTYPE;
 import static jdk.nashorn.internal.tools.nasgen.StringConstants.PROTOTYPEOBJECT_SETCONSTRUCTOR;
 import static jdk.nashorn.internal.tools.nasgen.StringConstants.PROTOTYPEOBJECT_SETCONSTRUCTOR_DESC;
 import static jdk.nashorn.internal.tools.nasgen.StringConstants.PROTOTYPEOBJECT_TYPE;
@@ -47,6 +46,8 @@
 import static jdk.nashorn.internal.tools.nasgen.StringConstants.SCRIPTFUNCTIONIMPL_TYPE;
 import static jdk.nashorn.internal.tools.nasgen.StringConstants.SCRIPTFUNCTION_SETARITY;
 import static jdk.nashorn.internal.tools.nasgen.StringConstants.SCRIPTFUNCTION_SETARITY_DESC;
+import static jdk.nashorn.internal.tools.nasgen.StringConstants.SCRIPTFUNCTION_SETPROTOTYPE;
+import static jdk.nashorn.internal.tools.nasgen.StringConstants.SCRIPTFUNCTION_SETPROTOTYPE_DESC;
 import static jdk.nashorn.internal.tools.nasgen.StringConstants.SCRIPTFUNCTION_TYPE;
 import static jdk.nashorn.internal.tools.nasgen.StringConstants.SCRIPTOBJECT_INIT_DESC;
 import static jdk.nashorn.internal.tools.nasgen.StringConstants.SCRIPTOBJECT_TYPE;
@@ -238,7 +239,7 @@
             mi.loadThis();
             mi.invokeStatic(PROTOTYPEOBJECT_TYPE, PROTOTYPEOBJECT_SETCONSTRUCTOR,
                     PROTOTYPEOBJECT_SETCONSTRUCTOR_DESC);
-            mi.putField(SCRIPTFUNCTION_TYPE, PROTOTYPE, OBJECT_DESC);
+            mi.invokeVirtual(SCRIPTFUNCTION_TYPE, SCRIPTFUNCTION_SETPROTOTYPE, SCRIPTFUNCTION_SETPROTOTYPE_DESC);
         }
     }
 
--- a/nashorn/buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/StringConstants.java	Fri Apr 26 18:31:42 2013 +0530
+++ b/nashorn/buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/StringConstants.java	Fri Apr 26 17:35:40 2013 +0200
@@ -55,7 +55,6 @@
     static final Type TYPE_SCRIPTFUNCTIONIMPL = Type.getType(ScriptFunctionImpl.class);
     static final Type TYPE_SCRIPTOBJECT       = Type.getType(ScriptObject.class);
 
-    static final String PROTOTYPE = "prototype";
     static final String PROTOTYPE_SUFFIX = "$Prototype";
     static final String CONSTRUCTOR_SUFFIX = "$Constructor";
     // This field name is known to Nashorn runtime (Context).
@@ -88,6 +87,8 @@
         Type.getMethodDescriptor(Type.VOID_TYPE, TYPE_STRING, TYPE_METHODHANDLE, TYPE_PROPERTYMAP, TYPE_METHODHANDLE_ARRAY);
     static final String SCRIPTFUNCTION_SETARITY = "setArity";
     static final String SCRIPTFUNCTION_SETARITY_DESC = Type.getMethodDescriptor(Type.VOID_TYPE, Type.INT_TYPE);
+    static final String SCRIPTFUNCTION_SETPROTOTYPE = "setPrototype";
+    static final String SCRIPTFUNCTION_SETPROTOTYPE_DESC = Type.getMethodDescriptor(Type.VOID_TYPE, TYPE_OBJECT);
     static final String PROTOTYPEOBJECT_TYPE = TYPE_PROTOTYPEOBJECT.getInternalName();
     static final String PROTOTYPEOBJECT_SETCONSTRUCTOR = "setConstructor";
     static final String PROTOTYPEOBJECT_SETCONSTRUCTOR_DESC = Type.getMethodDescriptor(Type.VOID_TYPE, TYPE_OBJECT, TYPE_OBJECT);
--- a/nashorn/src/jdk/nashorn/internal/objects/BoundScriptFunctionImpl.java	Fri Apr 26 18:31:42 2013 +0530
+++ b/nashorn/src/jdk/nashorn/internal/objects/BoundScriptFunctionImpl.java	Fri Apr 26 17:35:40 2013 +0200
@@ -40,7 +40,7 @@
 
     BoundScriptFunctionImpl(ScriptFunctionData data, ScriptFunction targetFunction) {
         super(data);
-        this.prototype = ScriptRuntime.UNDEFINED;
+        setPrototype(ScriptRuntime.UNDEFINED);
         this.targetFunction = targetFunction;
     }
 
--- a/nashorn/src/jdk/nashorn/internal/objects/NativeDebug.java	Fri Apr 26 18:31:42 2013 +0530
+++ b/nashorn/src/jdk/nashorn/internal/objects/NativeDebug.java	Fri Apr 26 17:35:40 2013 +0200
@@ -247,7 +247,6 @@
         out.println("Scope count " + ScriptObject.getScopeCount());
         out.println("ScriptObject listeners added " + PropertyListenerManager.getListenersAdded());
         out.println("ScriptObject listeners removed " + PropertyListenerManager.getListenersRemoved());
-        out.println("ScriptObject listeners dead " + PropertyListenerManager.getListenersDead());
         out.println("ScriptFunction count " + ScriptObject.getCount());
         out.println("ScriptFunction invokes " + ScriptFunction.getInvokes());
         out.println("ScriptFunction allocations " + ScriptFunction.getAllocations());
--- a/nashorn/src/jdk/nashorn/internal/objects/ScriptFunctionImpl.java	Fri Apr 26 18:31:42 2013 +0530
+++ b/nashorn/src/jdk/nashorn/internal/objects/ScriptFunctionImpl.java	Fri Apr 26 17:35:40 2013 +0200
@@ -42,6 +42,10 @@
  * function objects -- to expose properties like "prototype", "length" etc.
  */
 public class ScriptFunctionImpl extends ScriptFunction {
+
+    /** Reference to constructor prototype. */
+    private Object prototype;
+
     // property map for strict mode functions
     private static final PropertyMap strictmodemap$;
     // property map for bound functions
@@ -49,6 +53,9 @@
     // property map for non-strict, non-bound functions.
     private static final PropertyMap nasgenmap$;
 
+    // Marker object for lazily initialized prototype object
+    private static final Object LAZY_PROTOTYPE = new Object();
+
     /**
      * Constructor called by Nasgen generated code, no membercount, use the default map.
      * Creates builtin functions only.
@@ -83,8 +90,8 @@
      * @param methodHandle handle for invocation
      * @param scope scope object
      * @param specs specialized versions of this method, if available, null otherwise
-     * @param strict are we in strict mode
-     * @param builtin is this a built-in function
+     * @param isStrict are we in strict mode
+     * @param isBuiltin is this a built-in function
      * @param isConstructor can the function be used as a constructor (most can; some built-ins are restricted).
      */
     ScriptFunctionImpl(final String name, final MethodHandle methodHandle, final ScriptObject scope, final MethodHandle[] specs, final boolean isStrict, final boolean isBuiltin, final boolean isConstructor) {
@@ -235,10 +242,23 @@
         return Global.objectPrototype();
     }
 
+    @Override
+    public final Object getPrototype() {
+        if (prototype == LAZY_PROTOTYPE) {
+            prototype = new PrototypeObject(this);
+        }
+        return prototype;
+    }
+
+    @Override
+    public final void setPrototype(final Object prototype) {
+        this.prototype = prototype;
+    }
+
     // Internals below..
     private void init() {
         this.setProto(Global.instance().getFunctionPrototype());
-        this.setPrototype(new PrototypeObject(this));
+        this.prototype = LAZY_PROTOTYPE;
 
         if (isStrict()) {
             final ScriptFunction func = getTypeErrorThrower();
--- a/nashorn/src/jdk/nashorn/internal/runtime/PropertyListenerManager.java	Fri Apr 26 18:31:42 2013 +0530
+++ b/nashorn/src/jdk/nashorn/internal/runtime/PropertyListenerManager.java	Fri Apr 26 17:35:40 2013 +0200
@@ -25,20 +25,20 @@
 
 package jdk.nashorn.internal.runtime;
 
-import java.lang.ref.WeakReference;
-import java.util.ArrayList;
-import java.util.Iterator;
-import java.util.List;
+import java.util.Map;
+import java.util.WeakHashMap;
 
 /**
  * Helper class to manage property listeners and notification.
  */
 public class PropertyListenerManager implements PropertyListener {
 
+    /** property listeners for this object. */
+    private Map<PropertyListener,Boolean> listeners;
+
     // These counters are updated in debug mode
     private static int listenersAdded;
     private static int listenersRemoved;
-    private static int listenersDead;
 
     /**
      * @return the listenersAdded
@@ -54,16 +54,6 @@
         return listenersRemoved;
     }
 
-    /**
-     * @return the listenersDead
-     */
-    public static int getListenersDead() {
-        return listenersDead;
-    }
-
-    /** property listeners for this object. */
-    private List<WeakReference<PropertyListener>> listeners;
-
     // Property listener management methods
 
     /**
@@ -73,12 +63,13 @@
      */
     public final void addPropertyListener(final PropertyListener listener) {
         if (listeners == null) {
-            listeners = new ArrayList<>();
+            listeners = new WeakHashMap<>();
         }
+
         if (Context.DEBUG) {
             listenersAdded++;
         }
-        listeners.add(new WeakReference<>(listener));
+        listeners.put(listener, Boolean.TRUE);
     }
 
     /**
@@ -88,15 +79,10 @@
      */
     public final void removePropertyListener(final PropertyListener listener) {
         if (listeners != null) {
-            final Iterator<WeakReference<PropertyListener>> iter = listeners.iterator();
-            while (iter.hasNext()) {
-                if (iter.next().get() == listener) {
-                    if (Context.DEBUG) {
-                        listenersRemoved++;
-                    }
-                    iter.remove();
-                }
+            if (Context.DEBUG) {
+                listenersRemoved++;
             }
+            listeners.remove(listener);
         }
     }
 
@@ -108,18 +94,8 @@
      */
     protected final void notifyPropertyAdded(final ScriptObject object, final Property prop) {
         if (listeners != null) {
-            final Iterator<WeakReference<PropertyListener>> iter = listeners.iterator();
-            while (iter.hasNext()) {
-                final WeakReference<PropertyListener> weakRef = iter.next();
-                final PropertyListener listener = weakRef.get();
-                if (listener == null) {
-                    if (Context.DEBUG) {
-                        listenersDead++;
-                    }
-                    iter.remove();
-                } else {
-                    listener.propertyAdded(object, prop);
-                }
+            for (PropertyListener listener : listeners.keySet()) {
+                listener.propertyAdded(object, prop);
             }
         }
     }
@@ -132,18 +108,8 @@
      */
     protected final void notifyPropertyDeleted(final ScriptObject object, final Property prop) {
         if (listeners != null) {
-            final Iterator<WeakReference<PropertyListener>> iter = listeners.iterator();
-            while (iter.hasNext()) {
-                final WeakReference<PropertyListener> weakRef = iter.next();
-                final PropertyListener listener = weakRef.get();
-                if (listener == null) {
-                    if (Context.DEBUG) {
-                        listenersDead++;
-                    }
-                    iter.remove();
-                } else {
-                    listener.propertyDeleted(object, prop);
-                }
+            for (PropertyListener listener : listeners.keySet()) {
+                listener.propertyDeleted(object, prop);
             }
         }
     }
@@ -157,18 +123,8 @@
      */
     protected final void notifyPropertyModified(final ScriptObject object, final Property oldProp, final Property newProp) {
         if (listeners != null) {
-            final Iterator<WeakReference<PropertyListener>> iter = listeners.iterator();
-            while (iter.hasNext()) {
-                final WeakReference<PropertyListener> weakRef = iter.next();
-                final PropertyListener listener = weakRef.get();
-                if (listener == null) {
-                    if (Context.DEBUG) {
-                        listenersDead++;
-                    }
-                    iter.remove();
-                } else {
-                    listener.propertyModified(object, oldProp, newProp);
-                }
+            for (PropertyListener listener : listeners.keySet()) {
+                listener.propertyModified(object, oldProp, newProp);
             }
         }
     }
--- a/nashorn/src/jdk/nashorn/internal/runtime/ScriptFunction.java	Fri Apr 26 18:31:42 2013 +0530
+++ b/nashorn/src/jdk/nashorn/internal/runtime/ScriptFunction.java	Fri Apr 26 17:35:40 2013 +0200
@@ -71,9 +71,6 @@
 
     private static final MethodHandle IS_NONSTRICT_FUNCTION = findOwnMH("isNonStrictFunction", boolean.class, Object.class, Object.class, ScriptFunctionData.class);
 
-    /** Reference to constructor prototype. */
-    protected Object prototype;
-
     /** The parent scope. */
     private final ScriptObject scope;
 
@@ -221,6 +218,7 @@
         final ScriptObject object = data.allocate();
 
         if (object != null) {
+            Object prototype = getPrototype();
             if (prototype instanceof ScriptObject) {
                 object.setProto((ScriptObject)prototype);
             }
@@ -282,24 +280,18 @@
      * Get the prototype object for this function
      * @return prototype
      */
-    public final Object getPrototype() {
-        return prototype;
-    }
+    public abstract Object getPrototype();
 
     /**
      * Set the prototype object for this function
      * @param prototype new prototype object
-     * @return the prototype parameter
      */
-    public final Object setPrototype(final Object prototype) {
-        this.prototype = prototype;
-        return prototype;
-    }
+    public abstract void setPrototype(Object prototype);
 
     /**
      * Return the most appropriate invoke handle if there are specializations
      * @param type most specific method type to look for invocation with
-     * @param callsite args for trampoline invocation
+     * @param args args for trampoline invocation
      * @return invoke method handle
      */
     private MethodHandle getBestInvoker(final MethodType type, final Object[] args) {