Memory Management for nsIScriptContext: Difference between revisions

From MozillaWiki
Jump to navigation Jump to search
(Mention XULElement::GetCompiledEventHandler() copies the void *)
 
(10 intermediate revisions by 7 users not shown)
Line 1: Line 1:
=Script Object Lifetime Management=
=Script Object Lifetime Management=


This discusses the Script Object lifetimes for all script objects returned by the nsIScriptContext interface.  All such objects are returned as a "void *", and the lifetimes of these objects are implicitly tied up in the JS garbage collector and XPCWrappedNative lifetime management.  This document attempts to explicitly define lifetime rules such that they can be implemented by any language (well, by JS and Python in the first instance)
The problem:


==Script Globals==
Certain nsIScriptContext methods return a |void *| tied to the specific language.  For example, |CompileEventHandler| returns a "function" object which is held by the caller and later passed to |CallEventHandler|.  The lifetime of these returned |void *| objects must be managed.


At the high-level, it has been agreed that new methods |DropScriptObject| and |HoldScriptObject| will be added to certain language-specific interfaces.  The affected |nsIScriptContext| methods will return a 'held' object; the caller must call |DropScriptObject| when it no longer needs to store it.  |HoldScriptObject| is provided so that copies of the objects can be made.  This is identical to a reference counting scheme, but named to avoid confusion with XPCOMs reference counting.


A void * is obtained from the language for use as a "global" with an nsIScriptContext.  Currently it is assumed the lifetime of the object is the same as the lifetime of the context itself.  This model is reasonable and could be retained - however, if an alternate explicit mechanism is used in other places (see below), it may make sense to use that mechanism here.
We need to turn this into an actual implementation, with the following restrictions:
   
==CompileScript==


CompileScript returns a "void *" for the script object.  The interface comments for this function say:
* We want to avoid additional memory allocation or virtual method calls.  The only overhead the Drop/Hold scheme requires is one additional virtual-method call for the memory management function itself.


    The caller is responsible for GC rooting this object
* We can not have the memory management functions exclusively on nsIScriptContext.  There is code that currently stores a |void *| script object, but has no nsIScriptContext available.  One example is nsJSEventListener, which stores the |void *|, but has the |nsIScriptContext| provided as part of invoking the event.  nsJSEventListener would not want to store an nsIScriptContext just to free the script object.  Another example is the XUL cache which will be extended to cache all languages, but will not want to store an nsIScriptContext just to handle cache shutdown.
       
The only caller of this function (nsXULElement) does indeed ensure the void * returned is GC rooted.


As opposed to the event handlers discussed next, there is no "implied reference" for this return object - so the lifetimes of these objects must be explicitly managed.
[http://hostgator1centcoupon.net/ Hostgator VPS Coupon]
   
[http://hostgator-reseller-coupon.com/ Hostgator Reseller Coupon]
==Event Handlers==
[http://hostgator-vps-coupon.net/ Hostgator 1 cent coupon]


Event handlers are a little trickierThere are 3 steps to handling an event:
* Linkage issues mean that "helper functions" are unsuitable for use hereAs the DOM implementation and each language implementation are individual components, public static functions are not globally reachable.  Either xpcom interfaces or fully inline C++ classes can be used to assist with these tasks.


===Compile the script handler===
== Implementation idea #1 ==


Currently nsJSContext::CompileEventHandler does take the "event target" as a param - however, this is only used to setup principals, and does not otherwise "bind" the compiled function to the event target.  (Note: Brendan and I agreed that this param could be dropped from CompileEventHandler() to reinforce this lack of "binding", and the principals thing is apparently no longer necessary. Note however that CompileScript *does* still take an nsIPrincipal param - is that still necessary?)
Add DropScriptObect/HoldScriptObject to nsILanguageRuntime (a new class - a 'singleton' for each language - it creates nsIScriptContext objects amongst other things).  Code then only needs an integer language ID to be able to free the script object.


Due to this lack of binding, the "void *" returned by this function is identical to CompileScript above - the callers must ensure these unbound handlers are rooted if they hold on to them - there is no "implied reference" setup.
A downside of this approach is that all memory management functions take an extra penalty.  Each function involves:
* Fetching the nsDOMScriptObjectFactory service.
* Fetching the nsILanguageRuntime from the nsDOMScriptObjectFactory passing the int langID.
* Calling the nsILanguageRuntime::Drop/HoldScriptObject function.


===Bind the handler===
This problem could be solved by either:
* Duplicating these functions on nsIScriptContext, so code that does have a context available can do so without the additional overhead.
* Having performance critical code keep a reference to the nsILanguageRuntime.


This process binds the compiled event handler to a specific targetIn effect, this takes a copy of the compiled handler and "magically" associates it with the event target (see below).  After this process, the original compiled event handler is thrown away (and as the caller did not GCRoot the original, it will be disposed of by GC).  Critically however, this binding process has created an "implied reference" to the event target - once bound, other assumptions are then made that the event target will not be destroyed.
Even still, a key problem to this approach remains its fragilityBy explicitly requiring manual Drop/Hold calls, there is a lot of scope for programmer error in the face of error conditions (ie, early returns) to cause difficult to track leaksThe inevitability of such errors is evidenced by nsCOMPtr and various other tricks already in the code base.


===Call the bound handler===
== Implementation idea #2 ==


The "void *" passed to CallEventHandler is not the "void *" previously returned directly from CompileEventHandlerInstead, the "magic" association above is reversed, fetching a function object, by name, from the JS wrapper around the original event target.  Thus, currently the "void *" used is not supplied directly by nsIScriptContext, so the interface must be abstracted so it is.
This implementation builds on #1 as its core framework, and attempts to add a "helper class" (nsScriptObjectHolder) to automate the management of these objectsAll nsIScriptContext methods that return a "new" script object now take a reference to one of these objects.


==Notes:==
The public prototype for this class is:
Theoretically, the above setup should mean that an event handler for a specific object only requires the original event target and event name to fire the event.  The "magic" association would ensure that when querying for the event name, the previously cloned function object will be located.  However, even though this should be possible, it does not happen for XUL, which stores its XPCWrappedNative pointer in its nsJSEventListener.  Thus, XUL is now storing a weak reference to a different interface, not the event itself, in the assumption neither will go away.  XBL appears to store the original target.


The nsJSEventListener only keeps a borrowed reference to the target. Even though currently an XPCWrappedNative is stored in place of the original event, it is still a borrowed reference. This borrowed reference itself relies on the "implied reference" model below - without that implied reference, the weak reference to the object in nsJSEventListener would fail.  Note also that all of this "WrapNative" work happens *outside* nsIScriptContext, so we need to work out how to abstract it behind that interface.
  class nsScriptObjectHolder {
  public:
    nsScriptObjectHolder();
    nsScriptObjectHolder(PRUint32 aLangID, void *aObject);
    nsScriptObjectHolder(const nsScriptObjectHolder& other);
    ~nsScriptObjectHolder();
    nsScriptObjectHolder &operator=(const nsScriptObjectHolder &other);
    PRBool operator!() const;
    operator void *() const;
    nsresult set(PRUint32 langID, void *object);
    nsresult set(void *object);
    nsresult set(const nsScriptObjectHolder &other);
    nsresult clear();
    PRUint32 mLangID;
};


==The "implied reference"==
the nsIScriptContext methods changed are CompileScript, CompileEventHandler, GetBoundEventHandler, CompileFunction and Deserialize, with all changes similar to:


The above relies on an "implied reference" existing on the event target, via the "long-lived XPCWrappedNative" concept supported by XPConnect.  This mechanism is used by XPConnect tn ensure the same JS wrapper is always returned for a given DOM nsISupports.  While this model makes sense and all other languages would be encouraged to follow something similar, the nsIScriptContext model should not make any such implementation assumptions. Other languages may be able to perform the same magic using a different technique than XPConnect uses, and any "implied references" may not be true. For example, other languages may choose to implement this with weak references to make their own ownership model simpler.
    virtual nsresult GetBoundEventHandler(nsISupports* aTarget, void *aScope,
                                          nsIAtom* aName,
  -                                        void** aHandler) = 0;
  +                                        nsScriptObjectHolder &aHandler) = 0;


==Proposal==


===Unbound function objects:===
Code calling these methods changes from:
Unbound function objects (eg, returned by CompileScript, CompileEventHandler) should be managed by a simple unlock/lock memory management API:
  void *handler = nsnull;
  rv = context->GetBoundEventHandler(..., &handler);
  if (handler)
    context->CallEventHandler(..., handler)


* All such objects returned by nsIScriptContext are considered "locked".  They must remain locked while they are held externally. An 'unlock' function will be provided by the language to unlock the object.
to:
* It is likely that users of this interface may need to copy the returned object and manage the lifetimes seperately (for example, storing the returned "void *" in 2 unrelated datastructures).  For this scenario, an explicit "lock()" call will be provided (meaning 2 unlock calls are now needed to finalize the object).  However, as mentioned above, objects will always be returned locked so calling this will be rare.  XULElement::GetCompiledEventHandler() is the only identified place that does this.


===Bound Event Handlers===
  nsScriptObjectHolder handler;
  rv = context->GetBoundEventHandler(..., handler);
  if (handler)
    context->CallEventHandler(..., handler)


There are 2 options here - one relies on reusing the mechanism above for unbound functions, and the other relies on explicit xpcom strong references.
The use of "operator void *" and "operator !" means that much existing code that uses |void *| can remain unchanged once the variable has changed to nsScriptObjectHolder.  For example, an nsScriptObjectHolder can be passed to nsIScriptContext::CallEventHandler, even though the prototype remains |void *|.


NOTE: I'm still looking at the existing code trying to come up with a concrete proposal.
=== Problems ===


====Have BindCompiledEventHandler() return an out void **====
* The nsScriptObjectHolder class must have a fully inline implementation.  Any attempt to use methods defined in gklayout.dll mean the class can not be used by external languages (such as Python).


This may be the simplest - this would return a new void **, which must be passed to CallEventHandler().  This new void * is subject to the same rules as unbound handlers, and must be individually unlocked.
* This does not lend itself to allowing DropScriptObject/HoldScriptObject on both nsIScriptContext and nsILanguageRuntime.  As not all callers can use nsIScriptContext, it must be nsILanguageRuntime.  This means each memory management function is slightly expensive, as detailed above.


Language implementations are then free to have this "void *" indirectly hold a strong reference to the event target (or in the case of JS, need only ensure that the wrapper persists)
== Implementation idea #3 ==


====Formal binding objects====
Use a real XPCOM interface.  Something like:


The process of binding to an event handler can return a new nsISupports object. (As mentioned above, XUL already stores a different nsISupports pointer from the original event target - it just assumes that pointer has the same lifetime). A strong reference to this returned object must be kept and the "binding" is only guaranteed to remain alive as long as the object.  Once the binding has been dropped, a new Binding can be created, but it is not guaranteed the returned nsISupports will be the same.
interface nsIScriptObjectHolder : public nsISupports
{
    virtual void *GetScriptObject();
  }


As this is a strong reference, it will be necessary to ensure these references are manually droppedThis means that all nsJSEventListener objects must be explicitly dropped.  I believe this already happens now.
and all nsIScriptContext methods return one of these interfaces instead of a |void *|The memory management is then implicitly tied up in the lifetime of the interface itself.


===Global Script Objects===
=== Problems ===
* Fetching the |void *| language object becomes a virtual method.
* Each call requires a new allocation for the interface.


As mentioned at the start, it may (or may not) make sense to use the same explicit model we use for compiled scripts and event handlers.
== Notes ==
 
Implementation idea #2 has been implemented and works well.  However, concerns exist regarding the performance overhead in terms of speed and bloat.

Latest revision as of 18:47, 18 April 2012

Script Object Lifetime Management

The problem:

Certain nsIScriptContext methods return a |void *| tied to the specific language. For example, |CompileEventHandler| returns a "function" object which is held by the caller and later passed to |CallEventHandler|. The lifetime of these returned |void *| objects must be managed.

At the high-level, it has been agreed that new methods |DropScriptObject| and |HoldScriptObject| will be added to certain language-specific interfaces. The affected |nsIScriptContext| methods will return a 'held' object; the caller must call |DropScriptObject| when it no longer needs to store it. |HoldScriptObject| is provided so that copies of the objects can be made. This is identical to a reference counting scheme, but named to avoid confusion with XPCOMs reference counting.

We need to turn this into an actual implementation, with the following restrictions:

  • We want to avoid additional memory allocation or virtual method calls. The only overhead the Drop/Hold scheme requires is one additional virtual-method call for the memory management function itself.
  • We can not have the memory management functions exclusively on nsIScriptContext. There is code that currently stores a |void *| script object, but has no nsIScriptContext available. One example is nsJSEventListener, which stores the |void *|, but has the |nsIScriptContext| provided as part of invoking the event. nsJSEventListener would not want to store an nsIScriptContext just to free the script object. Another example is the XUL cache which will be extended to cache all languages, but will not want to store an nsIScriptContext just to handle cache shutdown.

Hostgator VPS Coupon Hostgator Reseller Coupon Hostgator 1 cent coupon

  • Linkage issues mean that "helper functions" are unsuitable for use here. As the DOM implementation and each language implementation are individual components, public static functions are not globally reachable. Either xpcom interfaces or fully inline C++ classes can be used to assist with these tasks.

Implementation idea #1

Add DropScriptObect/HoldScriptObject to nsILanguageRuntime (a new class - a 'singleton' for each language - it creates nsIScriptContext objects amongst other things). Code then only needs an integer language ID to be able to free the script object.

A downside of this approach is that all memory management functions take an extra penalty. Each function involves:

  • Fetching the nsDOMScriptObjectFactory service.
  • Fetching the nsILanguageRuntime from the nsDOMScriptObjectFactory passing the int langID.
  • Calling the nsILanguageRuntime::Drop/HoldScriptObject function.

This problem could be solved by either:

  • Duplicating these functions on nsIScriptContext, so code that does have a context available can do so without the additional overhead.
  • Having performance critical code keep a reference to the nsILanguageRuntime.

Even still, a key problem to this approach remains its fragility. By explicitly requiring manual Drop/Hold calls, there is a lot of scope for programmer error in the face of error conditions (ie, early returns) to cause difficult to track leaks. The inevitability of such errors is evidenced by nsCOMPtr and various other tricks already in the code base.

Implementation idea #2

This implementation builds on #1 as its core framework, and attempts to add a "helper class" (nsScriptObjectHolder) to automate the management of these objects. All nsIScriptContext methods that return a "new" script object now take a reference to one of these objects.

The public prototype for this class is:

class nsScriptObjectHolder {
public:
   nsScriptObjectHolder();
   nsScriptObjectHolder(PRUint32 aLangID, void *aObject);
   nsScriptObjectHolder(const nsScriptObjectHolder& other);
   ~nsScriptObjectHolder();
   nsScriptObjectHolder &operator=(const nsScriptObjectHolder &other);
   PRBool operator!() const;
   operator void *() const;
   nsresult set(PRUint32 langID, void *object);
   nsresult set(void *object);
   nsresult set(const nsScriptObjectHolder &other);
   nsresult clear();
   PRUint32 mLangID;
};

the nsIScriptContext methods changed are CompileScript, CompileEventHandler, GetBoundEventHandler, CompileFunction and Deserialize, with all changes similar to:

   virtual nsresult GetBoundEventHandler(nsISupports* aTarget, void *aScope,
                                         nsIAtom* aName,
-                                        void** aHandler) = 0;
+                                        nsScriptObjectHolder &aHandler) = 0;


Code calling these methods changes from:

 void *handler = nsnull;
 rv = context->GetBoundEventHandler(..., &handler);
 if (handler)
   context->CallEventHandler(..., handler)

to:

 nsScriptObjectHolder handler;
 rv = context->GetBoundEventHandler(..., handler);
 if (handler)
   context->CallEventHandler(..., handler)

The use of "operator void *" and "operator !" means that much existing code that uses |void *| can remain unchanged once the variable has changed to nsScriptObjectHolder. For example, an nsScriptObjectHolder can be passed to nsIScriptContext::CallEventHandler, even though the prototype remains |void *|.

Problems

  • The nsScriptObjectHolder class must have a fully inline implementation. Any attempt to use methods defined in gklayout.dll mean the class can not be used by external languages (such as Python).
  • This does not lend itself to allowing DropScriptObject/HoldScriptObject on both nsIScriptContext and nsILanguageRuntime. As not all callers can use nsIScriptContext, it must be nsILanguageRuntime. This means each memory management function is slightly expensive, as detailed above.

Implementation idea #3

Use a real XPCOM interface. Something like:

interface nsIScriptObjectHolder : public nsISupports
{
   virtual void *GetScriptObject();
}

and all nsIScriptContext methods return one of these interfaces instead of a |void *|. The memory management is then implicitly tied up in the lifetime of the interface itself.

Problems

  • Fetching the |void *| language object becomes a virtual method.
  • Each call requires a new allocation for the interface.

Notes

Implementation idea #2 has been implemented and works well. However, concerns exist regarding the performance overhead in terms of speed and bloat.