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.

Sign up for GitHub

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

Jump to bottom

Foreach memory leak (Chrome and Edge) #2223

Closed
cosmanovich opened this issue Apr 6, 2017 · 13 comments
Closed

Foreach memory leak (Chrome and Edge) #2223

cosmanovich opened this issue Apr 6, 2017 · 13 comments

Comments

@cosmanovich
Copy link

I run into an issue when running my KnockoutJS v3.4.2 (test) application in Google Chrome. The memory usage of my page keeps increasing.

The test code is a very simple piece of code, that changes the items in an observable array every second:

HTML:

<html>
    <head>
        <title>KnockoutJS</title>
    </head>
    <body>
        <h1>Foreach test</h1>
        <ul id="ul-numbers" data-bind="foreach: { data: listOfItems }">
            <li>
                <span data-bind="text: $data"></span>
            </li>
        </ul>

        <script type="text/javascript" src="./lib/knockout.js"></script>
        <script type="text/javascript" src="./index.js"></script> 
    </body>
</html>

JavaScript:

var vm = {
    listOfItems: ko.observableArray()
};

window.setInterval(function updateList(){
    var array = [];

    for(var i = 0 ; i < 1000; i++){
        var num = Math.floor( Math.random() * 500);
        array.push(num);
    }

    vm.listOfItems(array);
}, 1000);

ko.applyBindings(vm);

Memory usage:

  • In Firefox the memory usage doesn't increase:

    start: 459.6 MB ---> After +- 1 hour: 279.4 MB

  • In chrome the memory usage keeps increasing (memory of individual tab):

    start: 52.912 MB ---> After +- 1 hour: 566.120 MB

  • In edge the memory usage also keeps increasing (memory of individual tab):

    start: 109.560 MB ---> After +- 1 hour: 385.820 MB

Am I doing something wrong in this code snippet? Or is it an issue in knockoutJS (or the browsers)?

Test project:
KnockoutJS.zip
@brianmhunt
Copy link
Member

Thanks for the report.

Would you mind checking if the problem occurs with the TKO alpha? (if you use ko.js you shouldn't have to change any code)

@cosmanovich
Copy link
Author

I think the behavior is the same with the other library. (Tested it in chrome)

This is the memory usage when opening the page:
image

And after just a few minutes, it's increased like this:
image

(The second column is the total memory, the last column is the JavaScript Memory)

@brianmhunt
Copy link
Member

Good to know, thanks for testing it.

That helps likely narrows it down to something in ObservableArray and not the foreach bindings (since tko and ko 3.4 have completely different foreach bindings).

@cosmanovich
Copy link
Author

No problem. If there is anything else I could test to help you, please ask.

If you want to test it yourself, I have added the test project in my first post (at the bottom).

@cosmanovich
Copy link
Author

I don't know if this is important, but when I don't use the foreach binding in my view the memory leak doesn't occur. So when I just update the observable without binding to it, the memory usage doesn't increase.

@brianmhunt
Copy link
Member

I'm seeing the memory increase in the tab, as expected, but I've not been able to diagnose the cause yet.

When I run this (with TKO), here's Chrome's Allocation Timeline statistics after running for 10 intervals:

screen shot 2017-04-08 at 8 58 07 am

And here for 40 intervals:

screen shot 2017-04-08 at 8 58 32 am

It would appear that memory allocation, at least of arrays, is not the cause of the increase.

The "System Objects" appears to be growing. I can only suspect that by saying "System" the inference is that it's not memory references that we control.

With some more time I might be inclined to look at the arrayChangeTracking behaviour – disabling the diff caching. Other than that I can only guess that there's a variable that can be set to null, or an internal issue/edge case with Chrome GC.

@miellaby
Copy link
Contributor

miellaby commented Apr 8, 2017

https://jsfiddle.net/q93cgv4w/

With Chrome Timeline tool, I see an ever growing amount of DOM Nodes. Curiously,the heap snapshot tool doesn't show a growing amount of resources, not even detached DOM entries.

@cosmanovich
Copy link
Author

@miellaby I came to the exact same conclusion when I tested this, but I tought I could not find the real cause of this issue, because I'm not that familiar with the memory profiling tools (and the internals of Knockout).

But because the issue also occurs in Microsoft Edge I don't think it's a Google Chrome issue. What do you guys think?

@miellaby
Copy link
Contributor

@cosmanovich Or maybe ... https://bugs.chromium.org/p/chromium/issues/detail?id=709499

@cosmanovich
Copy link
Author

@miellaby That looks like the problem we are having :). So there probably is an issue in the Knockout library, but at the moment it will be very difficult to find out, what causes the issue.

What do you suggest, wait until that bug in Chrome is resolved? Or should we test it in Edge?

@brianmhunt
Copy link
Member

From the bug report, it looks like the profiler was working in Chrome/M45. Going back to that version might yield some indication of the leak's origin.

@ghost
Copy link

ghost commented May 18, 2017

Hi guys, my 2 cents.
I encountered the same problem in my application (2 nested foreach loop) but after the update of chrome( now at version 58.0.3029.110) knockout seems to work correctly and the memory doesn't increase indefinitely anymore.

Hope it helps.

@cosmanovich
Copy link
Author

This issue is indeed solved in the new version of Chrome.
I retested the test project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@brianmhunt @miellaby @cosmanovich