Hi all
Apologies for a rather long post, please bear with me :) I’ve been thinking a lot lately about memory management in Qooxdoo apps, specifically how to reliably remove the need to call dispose() (without causing leaks) and reducing complexity to our applications — I have a proposal for how to achieve this but it requires some subtle changes to the Qooxdoo framework. In theory this would be largely backward compatible (with a few specific exceptions) and would make it significantly easier to write long-running or memory sensitive applications. Although the mantra is that “if you created it, you should dispose it”, this is increasingly hard to follow as an application becomes more complex. There are specific examples of why this is virtually impossible baked into Qooxdoo, for example, some classes override setXxxx methods to *copy* the passed in object rather than take ownership of it - which means that the calling code has to understand the private implementation of the object in order to correctly handle cleanup. qx.ui.form.SelectBox.setSelected is an example of this - the caller has to “know” that setSelected will copy the array it is given and not take it over and be responsible for disposing it. Another common example with qx.data.Array is that methods such as splice and slice return an array of the removed items, but the returned object is an instance of qx.data.Array so you *must* dispose it. To illustrate my point, take a look at these samples of code and try to determine which is the “correct” one: Sample 1A: var selection = // … snip: get an array (.. or is it a qx.data.Array ..?) selection.slice(0,1); // delete the first element Sample 1B: var selection = // … snip: get an array (.. or is it a qx.data.Array ..?) selection.slice(0,1).dispose(); // delete the first element In order to figure out which piece of code is correct, you must understand the “…snip: get an array” and know if it is a native array or a qx.data.Array How about these examples: Sample 2A: var widget = // … snip: get a widget var selection = new qx.data.Array(); widget.setSelection(selection); Sample 2B: var widget = // … snip: get a widget var selection = new qx.data.Array(); widget.setSelection(selection); selection.dispose(); Sample 2C: var widget = // … snip: get a widget var selection = new qx.data.Array(); var oldSelection = widget.getSelection(); widget.setSelection(selection); if (oldSelection) oldSelection.dispose(); In order to choose from those samples, you must understand what the widget object is, where it came from, _and_ how it implements setSelection to know whether and what to dispose. This is not good - not only is the answer obscure, but if the implementation of widget.setSelection changes, your code may break. Sample 2C is also much longer than 2A, so you’re more likely to create a bug when typing it, and of course more code === more testing work. My final example is simple, predictable layouts, EG: Sample 1: var widget = // .. snip get a widget widget.setLayout(new qx.ui.layout.VBox()); Sample 2: var widget = // .. snip get a widget var oldLayout = widget.getLayout(); if (oldLayout) oldLayout.dispose(); widget.setLayout(new qx.ui.layout.VBox()); Here, Sample 2 is arguably safer (given what we know about the implementation of setLayout) but is those 3 extra lines something that we will reliably type (and not make typos in, and actually test) every time we want to set the “layout” property value? What about properties less well known than “layout”? In all of these examples, there is no clear answer when reading the code as to who should handle the memory management and in many cases the work required to really satisfy the safe disposal of objects is difficult to know. To me, this is exactly why garbage collection features so prominently in modern languages. Why then does it seem like we’ve gone back a step with Qooxdoo and its reliance on the ObjectRegistry? Searching through the code reveals some possible explanations, but the comments for ObjectRegistry kind of says it all: /** * Registration for all instances of qooxdoo classes. Mainly * used to manage them for the final shutdown sequence and to * use weak references when connecting widgets to DOM nodes etc. * * @ignore(qx.dev, qx.dev.Debug.*) */ This says that the ObjectRegistry is there so that all un-disposed objects can be disposed, and so that there are weak references between DOM nodes and the Widgets they represent. IMHO it’s possible to show that the weak links from DOM objects are the only reason that matters. Searching the code, and ignoring any debug specific code (e.g. qx.core.Logger and qx.dev.*), the only things that depend on ObjectRegistry having a lookup of every object are qx.ui.core.Widget and qx.data.controller.Tree. Code elsewhere depends on a widget having a hashCode, but crucially *not* depending on that hashCode being listed in the ObjectRegistry in a big lookup table of everything. In my own code, I use an object’s toHashCode and use the result in a lookup table - but only as a key in my own map, and rarely (if at all) by passing to qx.core.ObjectRegistry.fromHashCode(). If true for others then this is an important point because it means that by only registering objects with ObjectRegistry that _need_ to be registered (i.e. Widgets), memory management issues can almost completely disappear. To test this, I modified qx.ui.core.Widget.construct, qx.html.Element.__flush, and qx.data.controller.Tree.bindProperty so that all qx.core.Object’s have a hashCode, but only Widgets are registered with ObjectRegistry. …and then re-ran the Qooxdoo framework test suite. At the end of the run only 413 objects were not disposed. Not much, right? But without this patch there were 4,500 objects unfreed. (OK it’s fair to say that unit tests are not required to free up resources that they consume, but it is striking how easy it is to have significant resource leaks that can be so easily countered by garbage collection) A potential issue is that with garbage disposal, there are no destructors. That’s not to say that Qooxdoo destructors will not work, but they are only called if the objects are explicitly disposed() - and if automatic memory management is the norm, its increasingly less likely that the destructors will ever get called. The disadvantages of this proposal is that it includes a change to a core part of the library and that therefore brings risk of issues; that destructors are no longer possible and that fromHashCode cannot be used without extra code are small but potentially important details. On the other hand, the potential benefits are significant in reduced development time and reduced coding errors - the work necessary to manually manage memory can be considerable, and very difficult to test especially when it’s dependent on user interaction. I’ve created a branch in my fork of Qooxdoo to experiment with this, which you can see here: https://github.com/johnspackman/qooxdoo/tree/automatic-memory-management The automatic-memory-management branch is from the current master so if you’re using Qooxdoo 5.x you should be able to switch to it and experiment without any other side effects. Any thoughts are welcome :) Regards John
------------------------------------------------------------------------------
_______________________________________________ qooxdoo-devel mailing list qooxdoo-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/qooxdoo-devel
