krawaller / kranium

Brains for Titanium
http://www.kraniumjs.com
Other
103 stars 13 forks source link

Windows created through kranium are not garbage collected #3

Open netpro2k opened 13 years ago

netpro2k commented 13 years ago

This is because they are retained in global cache for K.elsByName, K.elsById, and K.elsByClassName (and potentially elsewhere). For views, calling remove() will remove them from these caches, but there is currently no way to do this for windows.

I would think that calling close() on a window should remove it, and its subviews from the cache, so that they can be garbage collected.

close() should probably look something like this (not thoroughly tested)

// cleanUp is optional in case you intend to re-open the same window. 
// Not really sure how opening a window that has been cleaned should 
// be handled, but it should likely rebuild itself
close: function(cleanUp){ 
    var el = this[0];

    var index, arr;

    if(cleanUp){ // This is pulled from remove()
        (el.className||"").split(/\s+/).forEach(function(cls){
            if(cls && (index = K.elsByClassName[cls].indexOf(el)) != -1){
                K.elsByClassName[cls].splice(index, 1); 
            }
        });

        if((arr = K.elsByName[el._type]) && ((index = arr.indexOf(el)) != -1)){
            arr.splice(index, 1);
        }
        if(el._id && K.elsById[el._id]){
            delete K.elsById[el._id];
        }

        // should also clean up children here, though not sure how to get at the 
        // instantiated children, as this.children is just the data structure used to 
        // call K.create on....
    }

    el && el.close && el.close();

    return this;
}

On a related note, it is unclear how to call close() from inside a KUI component, ex

app.js

K({
    type: 'tabgroup',
    tabs: [{
        title: 'Foo',
        window: {
            type: 'foo'
        }
    }]
}).open();

foo.js

exports.Class = Window.extend({
    title: 'Foo',
    navBarHidden: false,
    init: function(o){
        var self = this;

        this.leftNavButton = {
            title: 'Close',
            click: function(e){
                // self.close(); // this does not work because self is an instance of Window, not a Z collection
                $(self).close(); // this does work, but causes a second instance of the window to be added to global caches
            }
        };

        this.rightNavButton = {
            title: 'Open',
            click: function(e){
                K({
                    type: 'shims'
                }).open('tab')
            }
        };

        this._super(o);
    }
}
krawaller commented 13 years ago

Thanks for the investigation! I'll look into this and get back to you!

sokki commented 12 years ago

+1 for garbage collector.

I started using xcode Instruments and saw, that most of the elements created by Kranium are not garbage collected and produce memory leaks. i.e. the todolist in the kranium-demo. Every time you add a todo, all tableviewrows+1 are added to the living objects. 4,9,15,22,30 etc...

krawaller commented 12 years ago

Yes, this is a major problem, and I have yet to come up with a viable solution. I don't see how this could possibly be solved elegantly in the JS context. There is some research being done on WeakMaps (https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/WeakMap), which could help, but they're very very far from done yet.

I'm currently working on Kranium 0.2, and what I think I'll do there is to only make elements queryable if the developer explicitly says so, through setting the queryable property to true when creating the element. Of course, we'd still have to make sure any queryable elements were properly dereferenced when removed.

sokki commented 12 years ago

Thats a great Idea. Querying is "just" a nice-to-have feature for me. I love the kss-styling most. Without the need to rebuild the entire project like with standard-jss...

When do you think to finish the new version? I'm getting troubles with memory leaks and dont want to drop kranium.

krawaller commented 12 years ago

Good! Then I'll proceed making querying opt-in :-)

For the status of Kranium 0.2 and its Ti SDK 1.8 support, please see https://github.com/krawaller/kranium/issues/12

ziolmar commented 12 years ago

Please if anyone have solution for memory leaks in version 1.4 place it here. This is very important for us also.