0

I am looping over a response that I got from an ajax response, the response is an array containing 1000 objects , this response is used to created an html table with 1000 rows :

1st scenario :

    for (var i in msg.myObjects) {
    $('#mytablebody').append('<tr><td>' + msg.myObjects['item1'] + '</td><td>' + 
msg.myObjects['item2'] + '</td><td>' + msg.myObjects['item3'] + '</td><td>' + 
msg.myObjects['item4'] + '</td><td>' + msg.myObjects['item5'] + '</td><td>' + 
msg.myObjects['item6'] + '</td><td>' + msg.myObjects['item7'] + '</td> .... </tr>');
    }

Result => memory leak my RAM went to 2Go and my browser crashed

2nd scenario :

    for (var i in msg.myObjects) {
        document.getElementById('mytablebody').innerHTML = document.getElementById('mytablebody').innerHTML + '<tr><td>' + 
msg.myObjects['item1'] + '</td><td>' + 
    msg.myObjects['item2'] + '</td><td>' + msg.myObjects['item3'] + '</td><td>' + 
    msg.myObjects['item4'] + '</td><td>' + msg.myObjects['item5'] + '</td><td>' + 
    msg.myObjects['item6'] + '</td><td>' + msg.myObjects['item7'] + '</td> .... </tr>';
        }

Result => memory leak my RAM went to 800Mo and my browser crashed with a second ajax call

3rd scenario :

var stringResponse = '';
        for (var i in msg.myObjects) {
            stringResponse += '<tr><td>' + msg.myObjects['item1'] + '</td><td>' + 
        msg.myObjects['item2'] + '</td><td>' + msg.myObjects['item3'] + '</td><td>' + 
        msg.myObjects['item4'] + '</td><td>' + msg.myObjects['item5'] + '</td><td>' + 
        msg.myObjects['item6'] + '</td><td>' + msg.myObjects['item7'] + '</td> .... </tr>';
            }
document.getElementById('mytablebody').innerHTML = stringResponse 

Result => no memory leak

Ok until here I concluded that, first of all, .append() causes memory leaks, and second of all , you should never play with DOM elements inside a loop. But when I did the 4th scenario I concluded that the first conclusion was wrong (not exactly correct) and the second one is still correct.

4th scenario :

    var stringResponse = '';
            for (var i in msg.myObjects) {
                stringResponse += '<tr><td>' + replaceNulls(msg.myObjects['item1']) + '</td><td>' + 
            msg.myObjects['item2'] + '</td><td>' + msg.myObjects['item3'] + '</td><td>' + 
            msg.myObjects['item4'] + '</td><td>' + msg.myObjects['item5'] + '</td><td>' + 
            msg.myObjects['item6'] + '</td><td>' + msg.myObjects['item7'] + '</td> .... </tr>';
                }
    document.getElementById('mytablebody').innerHTML = stringResponse 

function replaceNulls(input) {
  return input != null ? input : ''
}

Result => memory leak my RAM went to 2Go and my browser crashed

My questions are:

when we call functions that occurs outside a loop , it may causes a memory leak, why ?

How can I avoid this (without removing the function or moving its processing to inside the loop) ?

Mehdi Souregi
  • 3,153
  • 5
  • 36
  • 53
  • Try pushing the `html` strings in an array and use `Array.join()` method when assigning it to the `innerHTML` – Sang Suantak Jan 24 '18 at 11:51
  • 1
    The issue is simply because you're making thousands of DOM operations in a loop, very quickly. Without knowing anything about the internals of the browser's JS engine, it's safe to say that's less than ideal. This is why the 3rd method you created (building the HTML string in memory, then appending it in a single DOM operation) is the best performing. This is the way all DOM appends *should* be done, but in most cases they are such small scale that any performance issues are not noticed. – Rory McCrossan Jan 24 '18 at 11:52
  • `it may causes a memory leak, why ?` There is no memory leak here.. – Keith Jan 24 '18 at 11:53
  • @RoryMcCrossan yes I agree , but i am stuck now with the 4th scenario, i can not use functions within a loop – Mehdi Souregi Jan 24 '18 at 11:53
  • The behavior you are experiencing is quite normal. You are spawning 1000 DOM elements. I dont know what your specific project needs are but I would suggest rethinking your UI by putting a button to limit the objects on the screen or using lazy loading – Kristiyan D Kovachev Jan 24 '18 at 11:54
  • @MehdiSouregi try: `'' + (msg.myObjects['item1'] || '') + ''` – Rory McCrossan Jan 24 '18 at 11:55
  • @RoryMcCrossan I agree , but I have two other methods of date conversion, i can remove the functions , but why I can not use them within a loop, why it causes memory leak – Mehdi Souregi Jan 24 '18 at 11:57
  • Which browser are you using? Have you tested in multiple browsers? I agree with @KristiyanDKovachev though, the main problem is weight of data. Is there no way you can using filtering or paging in the result set? 1000 entities is far too much for a user to process anyway – Rory McCrossan Jan 24 '18 at 11:59
  • You can also create the DOM nodes on a detached Node, then attach when filled in, this is much faster, and allows you to continue to use append etc. A 1000 lines in a table should not be any issue for any modern browser. – Keith Jan 24 '18 at 12:00
  • Chrome browser , i can make a fiddler , it is always the same result. @Keith my client want a page with 1000 entries. On page load there is no problem, 1000 entries are displayed and my browser RAM is on 200Mo and everything is fine, but when i call the server with ajax to get the second page i get memory leak when i use functions inside loop – Mehdi Souregi Jan 24 '18 at 12:04
  • if I remove the function replaceNull, everything is fine, my browser takes only 200MO RAM – Mehdi Souregi Jan 24 '18 at 12:05

1 Answers1

2

1000 table entries for modern web browsers should not cause any issue.

Here I'm adding 10,000 items into a table without ram problems. The reason why it's fast is because I build up the list inside a detached DOM element, then attach when done.

Your problem might be just down to DOM draw issue, the browser having to redraw on all your updates.

Another thing I noticed, for (var i in msg.myObjects) depending on what your msg.myObjects contains this is not a good thing to use. If you can use modern JS,. for (const i of msg.myObjects) is better.

var table = document.querySelector("table");

var tbody = document.createElement("tbody");

function addLine (txt) {
  var tr = document.createElement("tr");
  var td = document.createElement("td");
  tr.appendChild(td);
  td.innerText = txt;
  tbody.appendChild(tr);
}

for (var l = 1; l <= 10000; l += 1) {
  addLine("This is Line " + l + ", and some extra text");
}

table.appendChild(tbody);
<table>
  <thead><tr><th>Test</th></tr></thead>

</table>
Keith
  • 22,005
  • 2
  • 27
  • 44
  • let me test your idea and i will tell you the result , but i am sure it will cause a memory leak for my case , because i have 14 colmuns full of data – Mehdi Souregi Jan 24 '18 at 12:12
  • I've tried to create 14 nodes on jsfiddle and append them on the table body , it did work great , i will test it now on my code and Ill tell you the result right after – Mehdi Souregi Jan 24 '18 at 12:21
  • aand you are right, creating a node with each iteration was the perfect solution , if you have more information about why should i use const instead of using var it would be great , otherwise thank you for your help – Mehdi Souregi Jan 24 '18 at 13:04
  • 1
    `why should i use const instead of using var` It's not so much the `const`, but it's the `for in`, it will also iterate none owned properties, it's the reason you see a lot of `obj.hasOwnProperty(prop)` when `for in` is often used, `for of` does not have this issue. `const` is also a good habit for use in modern JS engines, for performance & linting. – Keith Jan 24 '18 at 13:14
  • 1
    Okay now I completely understood the difference between "for in" and "for of", i tried to create an array prototype outside this scope, and when I used the "for in" loop it is shown up like if it was an element on my array, and when I used the "for of" it is not shown up. so the "for of" is more usefull here. +1 – Mehdi Souregi Jan 24 '18 at 14:53
  • there is still a problem https://jsfiddle.net/7a9dzqnh/ , before clicking go watch your browser ram, click on the button 'OK' 10-15 times, and see how RAM is increasing over time – Mehdi Souregi Jan 24 '18 at 16:30
  • Well it will keep increasing, your adding another 10,000 lines each time you press the OK button. You need to remove the old TBODY, if you just want to show the current 10,000.. – Keith Jan 24 '18 at 16:54
  • 1
    Placing this at the begining of your click function will remove the old tbody, -> `var otoby = table.querySelector("tbody");if (otoby) otoby.remove();` – Keith Jan 24 '18 at 16:58
  • took me time to figure it out whats wrong with my code, the remove you suggested is correct , but what was not working perfectly on my code, i did a memory snapshot on chrome (i just learned how to do it) and i discovered that some cells on my table are not removed from memory , why ? because there is a method that is making those cells draggable (draggable clone from jquery-ui) , and it adds a class 'ui-draggable' and i guess when i remove the dom element i remove the wrong one ! Thank you for your help, I would not have resolved this problem without you :) – Mehdi Souregi Jan 25 '18 at 10:03