I think the whole concept of ObjectRegistry should be dropped. It creates nothing but overhead and it's really annoying to manually dispose objects. I remember that these weak references to connect DOM elements and widgets were created more than 10 years ago to support IE6, which is now dead. There is no modern browser that has problem with this anymore.
On Sat, Jan 2, 2016 at 1:16 PM, Varol Okan <mym...@movingsatellites.com> wrote: > John, > > I think this analysis is a great. > I have not looked at your changes but from this e-mail they seem small. > > I know that most JS developers will not actively manage the memory in > their applications, only the good developers do, or if a memory leak is > obviously causing harm. > > If we could get this integrated in QooxDoo, maybe with a switch to enable > this feature. E.g. It could be added as a switch during the compilation it > would not affect existing projects unless they would chose to enable this > feature. > > The when version 6.0 comes around it could be enabled per default. > > Cheers, > > Varol :) > > > > On 01/02/2016 06:12 AM, John Spackman wrote: > > 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 > listqooxdoo-devel@lists.sourceforge.nethttps://lists.sourceforge.net/lists/listinfo/qooxdoo-devel > > > > > ------------------------------------------------------------------------------ > > _______________________________________________ > qooxdoo-devel mailing list > qooxdoo-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/qooxdoo-devel > >
------------------------------------------------------------------------------
_______________________________________________ qooxdoo-devel mailing list qooxdoo-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/qooxdoo-devel
