Strange memory leak in knockout mapping plugin
Asked Answered
C

1

6

Can't figure out why disposing computed observables doesn't remove the subscriptions from global variables in case view model was created using knockout.mapping plugin.
First let's see what happens when model is created directly:

// Global variable.
var Environment = {
    currencyStr: ko.observable("usd.")
};
// Item model, used intensively.
function ItemModel(price) {
    var self = this;
        
    this.price = ko.computed(function () {
        // Computed is subscribed to global variable.
        return price + ' ' + Environment.currencyStr();
    });
};

ItemModel.prototype.dispose = function () {
    // Dispoing subscription to global variable.
    this.price.dispose();
};

function ViewModel() {
    var self = this;
    
    self.items = ko.observableArray([]);
    // Simply adds 1000 new items to observable array directly.
    self.addItems = function () {
        for (var i = 0; i < 1000; i++) {
            self.items.push(new ItemModel(i));
        }
    };
    // Disposes and removes items from observable array
    this.removeItems = function () {
        ko.utils.arrayForEach(self.items(), function (item) {
            item.dispose();
        });
        self.items.removeAll();
    };		
};

ko.applyBindings(new ViewModel());
<script src="https://cdnjs.cloudflare.com/ajax/libs/knockout/3.2.0/knockout-min.js"></script>
<button data-bind="click: addItems">Add items</button>
<button data-bind="click: removeItems">Remove items</button>
<div data-bind="foreach: items">
    <div>
        <span data-bind="text: price"></span>
    </div>
</div>

I used Chrome dev tools to record heap allocations while adding and removing items several times. After each addition, previously allocated objects were cleaned up successfully, I got the following picture:

enter image description here

Now the same functionality using mapping plugin:

// Global variable.
var Environment = {
    currencyStr: ko.observable("usd.")
};
// Item model, used intensively.
function ItemModel(price) {
    var self = this;
        
    this.price = ko.computed(function () {
        // Computed is subscribed to global variable.
        return price + ' ' + Environment.currencyStr();
    });
};

ItemModel.prototype.dispose = function () {
    // Dispoing subscription to global variable.
    this.price.dispose();
};

function ViewModel() {
    var self = this;
    
    self.items = ko.observableArray([]);
    self.itemsMapping = {
        'create': function (options) {
            return new ItemModel(options.data);
        }
    };
    // Simply adds 1000 new items to observable array using mapping plugin.
    self.addItems = function () {
        var itemsPrices = new Array(1000);
        for (var i = 0; i < 1000; i++) {
            itemsPrices[i] = i;
        }
        // Mapping new array to our observable array.
        ko.mapping.fromJS(itemsPrices, self.itemsMapping, self.items);
    };
    // Disposes and removes items from observable array
    this.removeItems = function () {
        ko.utils.arrayForEach(self.items(), function (item) {
            item.dispose();
        });
        self.items.removeAll();
    };		
};

ko.applyBindings(new ViewModel());
<script src="https://cdnjs.cloudflare.com/ajax/libs/knockout/3.2.0/knockout-min.js"></script>
<script src="//cdnjs.cloudflare.com/ajax/libs/knockout.mapping/2.4.1/knockout.mapping.min.js"></script>
<button data-bind="click: addItems">Add items</button>
<button data-bind="click: removeItems">Remove items</button>
<div data-bind="foreach: items">
    <div>
        <span data-bind="text: price"></span>
    </div>
</div>

Using the same technique to record heap allocations this is what I see:

enter image description here

I am aware of pureComputed, but would like to avoid using them for 2 reasons:

  1. Switching to pure computed breaks legacy code by throwing exceptions:

    'A 'pure' computed must not be called recursively

Solving these issues will take a lot of time.

  1. Pure computeds are evaluated more often which creates some performance overhead I'd like to avoid, and again this influences legacy code unpredictably.

Also I would still like to use the mapping plugin because of it's ability to monitor collection state (using key mapping property) and because it creates all observables for me.

So is there something that I missed and what is the proper way to free resources in case of using mapping plugin?

Cateyed answered 15/12, 2014 at 18:16 Comment(0)
G
5

Digging in to the mapping plugin, it does some hacking of the computed and evidently breaks it in this case.

It looks like setting your price computed to deferEvaluation makes the mapping plugin largely leave it alone.

this.price = ko.computed(function () {
    // Computed is subscribed to global variable.
    return price + ' ' + Environment.currencyStr();
}, this, { deferEvaluation: true });
Germinate answered 15/12, 2014 at 20:20 Comment(4)
Wow, that is pretty unexpected. I'll give it a try tomorrow and let you know the results, thanks!Cateyed
Couldn't wait. It really seems that leak is gone. Could you point the line number (or exact line of code to look for) in mapping plugin where you spotted the break of computed behavior? Thanks again!Cateyed
I couldn't pin down exactly why it breaks, but the code is here github.com/SteveSanderson/knockout.mapping/blob/master/… - It appears to be trying to set all computed to deferEvaluation because the properties they depend on may be loaded in the wrong order by the mapping plugin, so the first evaluation might fail.Germinate
I think I got the main idea - they want to create proxies for all computeds found in my view model (for the reason you mentioned). So when I try to dispose my computeds, it doesn't matter, because the proxy is disposed and not the real computed (at least I think so). But when we set deferEvaluation: true, they don't create proxy, so everything works as expected. Thanks, that is a good investigation!Cateyed

© 2022 - 2024 — McMap. All rights reserved.