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

the ko.mapping.toJS(viewmodel) memory leak #66

Closed
pierol opened this issue Apr 15, 2012 · 14 comments
Closed

the ko.mapping.toJS(viewmodel) memory leak #66

pierol opened this issue Apr 15, 2012 · 14 comments

Comments

@pierol
Copy link

pierol commented Apr 15, 2012

My view model is this:

 viewModel = {
        init: function () {
            $.ajax({
                type: 'GET',
                url: 'adminprodotto/init',
                async: false,
                contentType: 'application/json; charset=utf-8',
                success: function (data) {
                    $.extend(viewModel, ko.mapping.fromJS(data));
                    /*ko.utils.unwrapObservable(viewModel.Immagini);*/
                },
                error: function () {

                }
            });
        },
        vuoto: function () {
            $.ajax({
                type: 'GET',
                url: 'adminprodotto/init',
                async: false,
                contentType: 'application/json; charset=utf-8',
                success: function (data) {
                    ko.mapping.fromJS(data, viewModel)
                },
                error: function () {

                }
            });
        },
        save: function () {
            //alert(ko.mapping.toJSON(viewModel.Allegati));
            $.ajax({
                type: 'POST',
                url: 'adminprodotto/update',
                contentType: 'application/json; charset=utf-8',
                data: JSON.stringify(ko.mapping.toJS(viewModel)),
                success: function (data) {
                    $('#modal-prodotto').modal('hide');
                    tabellaProdotti.fnStandingRedraw();

                },
                error: function (jqXHR, textStatus, errorThrown) { alert('Errore:' + errorThrown) }
            });
        },
        edit: function (key) {
            $.ajax({
                type: 'GET',
                async: false,
                url: 'adminprodotto/detail?IdProdotto=' + key,
                contentType: 'application/json; charset=utf-8',
                success: function (data) {
                    ko.mapping.fromJS(data, viewModel);
                    $('#modal-prodotto').modal('show');
                },
                error: function (jqXHR, textStatus, errorThrown) { alert('Errore:' + errorThrown) }
            });
        },
        erase: function () {
            $.ajax({
                type: 'POST',
                url: 'adminprodotto/delete',
                async: false,
                data: JSON.stringify({ IdProdotto: viewModel.Prodotto.IdProdotto() }),
                contentType: 'application/json; charset=utf-8',
                success: function (data) {
                    $("#modal-delete-prodotto").modal('hide');
                    tabellaProdotti.fnStandingRedraw();
                },
                error: function (jqXHR, textStatus, errorThrown) { alert('Errore:' + errorThrown) }
            });
        },
        annulla: function () {
            tabellaProdotti.fnStandingRedraw();
        },
        removeImmagine: function (immagine, data) {
            immagine = ko.mapping.toJS(immagine);
            $.ajax({
                type: 'POST',
                url: '/file/deletefileimmagine',
                async: false,
                data: JSON.stringify({ Url: immagine.Url, IdProdottoImmagine: immagine.IdProdottoImmagine }),
                contentType: 'application/json; charset=utf-8',
                success: function (data) {
                    if (data == "ok") {
                        viewModel.Immagini.remove(function (item) {
                            return (item.IdProdottoImmagine() == allegato.IdProdottoImmaginr)
                        });
                    }
                },
                error: function (jqXHR, textStatus, errorThrown) { alert('Errore:' + errorThrown) }
            });
            return true; 
        },
        removeAllegato: function (allegato, data) {
            allegato = ko.mapping.toJS(allegato);
            $.ajax({
                type: 'POST',
                url: '/file/deletefileallegato',
                async: false,
                data: JSON.stringify({ Url: allegato.Url, IdProdottoAllegato: allegato.IdProdottoAllegato }),
                contentType: 'application/json; charset=utf-8',
                success: function (data) {
                    if (data == "ok") {
                        viewModel.Allegati.remove(function (item) {
                            return (item.IdProdottoAllegato() == allegato.IdProdottoAllegato)
                        });
                    }
                },
                error: function (jqXHR, textStatus, errorThrown) { alert('Errore:' + errorThrown) }
            });
            return true;
        }

    }

after five o six call to the "save" method of the viewmodel the call toJSON (i've tried with toJS same issue) cause a memory leak. I've tried the latest version of the plugin (debug version) and knockout (in chrome says thant the page is crashed, in firefox that a script doesn't respond anymore).

@sagacity
Copy link
Collaborator

Please try to isolate the problem to a small reproduction scenario you can put up on jsfiddle.net, otherwise it's very difficult to help you since this crash could be caused by a lot of things.

@pierol
Copy link
Author

pierol commented Apr 15, 2012

Yes i understand, but i never used jsfiddle.net and there are a lot of ajax call how could i do that ? I've tried to debug the code
of the library and i find the call that cause the memory leak but i don't know if this could help.

@sagacity
Copy link
Collaborator

What would already make it a lot simpler is if you replace the ajax calls with functions that just return the data. So, instead of doing:

$.ajax({success: function(data) { doSomething(data); }});

You could do:

var data = { a: 1, b: 2 };
doSomething(data);

So basically, not do the ajax call but just call the success function directly with data that would normally come from the ajax request.

@pierol
Copy link
Author

pierol commented Apr 15, 2012

Hi Roy, thank you very much for your suggestion. I made the fiddle example: http://jsfiddle.net/pierol/9MSSS/ the problem is that with fiddle i can simulate the problem because i can't open a modal which i think is the problem. In fact in the fiddle example if you repeatedly press the button "salva" the problem doesn't happen. If you want to give a try here the repository https://github.com/pierol/Knockout-memory-leak after 6/7 pressed to the "salva" button you will see the interface freezing and the browser telling to stop a script that doesn't respond.

@sagacity
Copy link
Collaborator

Hi Piero, you really need to narrow it down a bit more. Can you strip away all unneeded html? Also, if the problem is caused by opening a modal, then maybe that is the issue and NOT the mapping.

Have you tried posting on the Knockout google group? There are a lot of friendly people there who may be able to offer some suggestions as well: https://groups.google.com/forum/?fromgroups#!forum/knockoutjs

@pierol
Copy link
Author

pierol commented Apr 16, 2012

Hi Ryan, thank you again for your patient, after a narrow investiganting i've found that the problem is not the modal. I follow your suggestion and now i've semplified the html (https://github.com/pierol/Knockout-memory-leak). It comes out that the call to ko.mapping.toJSON(viewModel) is time consuming and after a sequence of clicks on the save button the script stopped to respond. In fiddle http://jsfiddle.net/pierol/9MSSS/ i can't reproduce the problem i think that the context in wich the sript is execute is different.

I will post the problem even to the google group but i'm quite sure that the problem is the mapping.

@sagacity
Copy link
Collaborator

Do you maybe have a lot more data in your 'real' scenario than in the JsFiddle? The mapping plugin does take time, so when you have a lot of items it can get slow.

@pierol
Copy link
Author

pierol commented Apr 16, 2012

The data in jsfiddle is identical to my real data. The problem i think is that repeating the toJSON operation more and more on same viewmodel perhaps the viewmodel grows even if the data remain the same. In fact the first 1 or 2 click the method call take a acceptable time and after the sixth o seventh click the call is too long and the page crash even if the data in the view model remain the same.

@sagacity
Copy link
Collaborator

Are you maybe creating subscriptions or computed/dependentObservables every time you open the modal, and not disposing of them? This way, every time you open the modal, you get more and more subscriptions, so that everything becomes slower and slower.

@pierol
Copy link
Author

pierol commented Apr 16, 2012

Every time i open the modal i made a ajax call to obtain the data in JSON format from the server then i make a ko.mapping.fromJS(datafromajaxCall,viewModel) to update the viewModel as i saw in the site documentation of the knockout library. I don't know if this call create internally subscriptions every time, i think it shouldn't.

$.ajax({
                type: 'GET',
                async: false,
                url: 'adminprodotto/detail?IdProdotto=' + key,
                contentType: 'application/json; charset=utf-8',
                success: function (data) {
                    ko.mapping.fromJS(data, viewModel);
                    $('#modal-prodotto').modal('show');
                },
                error: function (jqXHR, textStatus, errorThrown) { alert('Errore:' + errorThrown) }
            });

I create the viewModel in a non declarative way but making a ajax call and then extended the object viewModel with data1

  var viewModel = {
    init: function() {
        $.extend(viewModel, ko.mapping.fromJS(data1));
    },
    save: function() {
        ko.mapping.toJSON(viewModel);
    }
}
viewModel.init();
ko.applyBindings(viewModel);

but in the test html i made only sequence call to the ko.mapping.toJSON(viewModel) and the issue seems to come out in anyway without the fromJS call.

@pierol
Copy link
Author

pierol commented Apr 16, 2012

Investingating the source code of the mapping plugin i've found that this call inside the function ko.mapping.visitModel (line 621 of the knockout.mapping.latest-debug.js) is the time consuming function, every time i click on the save button this function become more and more time consuming even if the model is always the same:

visitPropertiesOrArrayEntries(unwrappedRootObject, function (indexer) {
            var currentOptions = options;
            var mappingObject = unwrappedRootObject[mappingProperty];
            if (mappingObject) {
                currentOptions = fillOptions(mappingObject,options);
            }

            if (currentOptions.ignore && ko.utils.arrayIndexOf(currentOptions.ignore, indexer) != -1) return;

            var propertyValue = unwrappedRootObject[indexer];
            options.parentName = getPropertyName(parentName, unwrappedRootObject, indexer);

            // If we don't want to explicitly copy the unmapped property...
            if (ko.utils.arrayIndexOf(currentOptions.copy, indexer) === -1) {
                // ...find out if it's a property we want to explicitly include
                if (ko.utils.arrayIndexOf(currentOptions.include, indexer) === -1) {
                    // The mapped properties object contains all the properties that were part of the original object.
                    // If a property does not exist, and it is not because it is part of an array (e.g. "myProp[3]"), then it should not be unmapped.
                    if (unwrappedRootObject[mappingProperty] && unwrappedRootObject[mappingProperty].mappedProperties && !unwrappedRootObject[mappingProperty].mappedProperties[indexer] && !(unwrappedRootObject instanceof Array)) {
                        return;
                    }
                }
            }

            var outputProperty;
            switch (ko.mapping.getType(ko.utils.unwrapObservable(propertyValue))) {
            case "object":
            case "undefined":
                var previouslyMappedValue = options.visitedObjects.get(propertyValue);
                mappedRootObject[indexer] = (ko.mapping.getType(previouslyMappedValue) !== "undefined") ? previouslyMappedValue : ko.mapping.visitModel(propertyValue, callback, options);
                break;
            default:
                mappedRootObject[indexer] = callback(propertyValue, options.parentName);
            }
        });

@sagacity
Copy link
Collaborator

Which version of the plugin are you using? (It's at the top of the file)

@pierol
Copy link
Author

pierol commented Apr 16, 2012

I'm using this

// Knockout Mapping plugin v2.0.4

@pierol
Copy link
Author

pierol commented Apr 16, 2012

And finally using this version

// Knockout Mapping plugin v2.1.1

the issue is solved Thank you Roy for your precious support. And sorry if i used an old version.

@pierol pierol closed this as completed Apr 16, 2012
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

2 participants