Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Windows created through kranium are not garbage collected #3

Open
netpro2k opened this issue Sep 24, 2011 · 6 comments
Open

Windows created through kranium are not garbage collected #3

netpro2k opened this issue Sep 24, 2011 · 6 comments

Comments

@netpro2k
Copy link
Contributor

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
Copy link
Owner

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

@sokki
Copy link

sokki commented Dec 28, 2011

+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
Copy link
Owner

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
Copy link

sokki commented Jan 3, 2012

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
Copy link
Owner

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 #12

@ziolmar
Copy link

ziolmar commented Jul 5, 2012

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants