How to properly optimize a long for loop that concats a long string everytime?

Question

I'm doing a small learning project, it takes a coin list from an API and then builds a page full of bootstrap card for the length of the array received from the API. While developing i only used 100 first coins to avoid long wait times, but now that I'm done when i try the entire 3900+ coins its takes an impractically long time.

I'm pretty sure the way i added the string is the source of the problem, I'll add my code and I'm sure it will all make more sense.

I tried building the entire string then append it - no good still slow. I tried changing it's innerHTML every time and then append it (append is inside the for loop), but it just overwrites all the older coins and appends only the last one.

What i want it to do is actually append each box separately and do it in a reasonable amount of time, right now it takes over 30 minutes to complete which is obviously not good.

The current version of code i added is the one that takes a long time but eventually does it right (although its only 50 iterations in the for loop so it doesn't get stuck if you try it right now, it needs to be over 3900 iterations)

function homeStart(coins: any[]): void {

  var divcreate = document.createElement("div");

  for (var i = 0; i < 50; i++) {
    // console.log(coins[i]);
    // ******This is where the string addup starts
    divcreate.innerHTML += `
            <div class="card text-dark bg-dark m-auto makeinline" 
            id="${coins[i].id}${i}" style="max-width: 18rem;">
            <div class="card-header">
                <div class="flexalign">
                    <span class="coinsymbol" 
            id="${coins[i].symbol.toUpperCase() + 
            "a1"}">${coins[i].symbol.toUpperCase()} 
           </span>
                    <label class="switch">
                        <input type="checkbox" 
         id="${coins[i].id}${coins[i].symbol}" 
        onchange="selectedCoinUpdate(this,'${coins[i].symbol}')"> 
                        <span class="slider round"></span>
                    </label>
                </div>
            </div>
            <div class="card-body">
                <div class="">
                    <h5 class="card-title coinname">${coins[i].name}</h5>
                    <button type="button" class="btn btn-secondary" data- 
       toggle="collapse" href="#collapseIdentity${i}" role="button"
                          aria-expanded="false" aria- 
       controls="collapseIdentity${i}" 
       onclick="moreInfo('${coins[i].id}')">More info</button>
                </div>
                    <div class="collapse" id="collapseIdentity${i}">
                        <div class="card-body" id="${coins[i].id}">

                        </div>
                    </div>
                </div>
            </div>`;

  }
  // ******This is where the string addup ends
  $("#pagecont").append(divcreate);

}

Thank you I will try it RIGHT NOW, cant believe i did something that silly, will report in a couple of minutes with result — adame21, Feb 05 '19 at 13:55
if those elements that you're trying to produce are not visible all at once, you may consider producing them by small portions as you scroll the list — , Feb 05 '19 at 13:55
Each time you do this -> `$("#pagecont").append(divcreate);` Your likely causing a document reflow, one way around this is to attach the elements to a detached div, and then attach at the end, causing a single document reflow. As an example -> https://stackoverflow.com/questions/48421790/memory-leak-in-javascript-functions-inside-loop/48422143#48422143 — Keith, Feb 05 '19 at 13:57
Manipulating and changing the DOM is really really slow in comparison of dealing with pure JavaScript. So do all you can in js without touching the Dom and at the end insert it in the DOM. And you can try to use the profile tab of the chrome Dev tools as well as the performance API to see what are the operations that take the more time — BenoitVasseur, Feb 05 '19 at 13:58
Ok, so someone suggested concatenating it into a variable first and it works a million times better now, but the problem is it still gets the page stuck and doesent show the lading circle while processing, any way around that? — adame21, Feb 05 '19 at 13:58
`page stuck`, Although your code is not asynchrouse, if you make it async, say by using `async / await`, this will actually prevent the UI locking up, as an `await` will allow a next tick event to fire. — Keith, Feb 05 '19 at 14:01
@Keith So i just moved the loader stop function a bit and its better, but could you elaborate on the changes required to implement what you said? where do i put async / await? thanks. EDIT: just to add, when i open it on live server it loads pretty and ok, when i refresh it gets stuck until it pops — adame21, Feb 05 '19 at 14:04
I'll see if I can knock up a quick snippet demonstrating the `async / await` to prevent `sync` loops lookup up the UI. — Keith, Feb 05 '19 at 14:16
Something's wrong with your UI design if you want to show 3900 items on a page. Nobody's gonna look at all of them. — Bergi, Feb 05 '19 at 15:58
@Bergi I understand what you are saying, but it is a project requirement, I agree its stupid but thats what they want. also if i were to append more items everytime you scroll to the bottom it would be counter-productive to the project's goal — adame21, Feb 05 '19 at 17:08

score 0 · Accepted Answer · answered Feb 05 '19 at 14:39

For loading DOM elements fast, you can see this link -> Memory leak in javascript, functions inside loop

After getting things as fast as you can, it still might be too slow and potentially cause the browser to freeze / crash.

Long running sync operation in JS will lock up the UI,.

To prevent this you can make a long running sync operation into async operation, this has the benefits of giving the UI some breathing space. You could then create a loading indicator etc.

Below is a really simple example, I'm doing a sync operation that takes about 100ms, and doing this 100 times. This would then add up to a total of 10 seconds, if you did this without using async it wouldn't be a very nice user experience.

To prove that the UI is not locked up, I'm then just flashing the Loading indicator pink, of course you could make a spinning circle etc. After it's finished it also console logs all done,..

Hopefully, this little example will help with long running sync operations.

ps: I always believed async/await was meant to do a next tick, for me though doing the loop without the sleep(0), would still cause the UI to lock up for 10 seconds.

const loaddiv = document.querySelector(".loading");

var i = setInterval(() => {
  loaddiv.classList.toggle("loading");  
}, 300);

const sleep = (time) => new Promise((resolve) => setTimeout(resolve, time));

async function blockUI() {
  let now = new Date().getTime();
  const desiredTime = now + 100;
  while (now < desiredTime) {
    now = new Date().getTime(); 
  }
  await sleep(0); //give the UI some breathing space
}

async function longLoop() {
  for (let l = 0; l < 100; l += 1) 
    await blockUI();
  clearInterval(i);  
  console.log("all done");
}

longLoop();

.loading {
  background-color: pink;
}

<div class="loading">
  Loading.
</div>

Thank you, really expanded on my question, and gave me something to implement into my project to make it smoother! appreciated, marked as answer. — adame21, Feb 05 '19 at 15:29
"*I always believed async/await was meant to do a next tick*" - no, promise callbacks execute as soon as possible for max speed, in a tight loop (but still perceived as asynchronous to the code). See "microtask" for details. To give the browser some time to reflow and repaint, you need to use an explicit timeout - which is however trivially integrated anywhere in async code with `await`. — Bergi, Feb 05 '19 at 15:57
@Bergi Yeah, makes sense, & also good to know. I think it's threads like -> https://stackoverflow.com/questions/27647742/promise-resolve-then-vs-setimmediate-vs-nexttick that led me to this idea. That was back in 2014, so I'm wondering if things have got more ratified since then, especially now since Promises are much more widely used, and waiting for nextTick could really hamper things now. — Keith, Feb 05 '19 at 19:09
@Keith I don't think anything changed there, but that question is about node.js specifically. — Bergi, Feb 05 '19 at 19:13

score 0 · Answer 2 · answered Feb 05 '19 at 20:16

As Keith said, you can transform your unique sync operation in multiple tasks and run those tasks in an async way (setTimeout in his example).

Doing this way instead of one big operation improve the UI performance but it is not that great because you touch the DOM.

Touching the DOM is slow because the browser may have lot of operations to do (paint, reflow, etc). So it is better to ask the browser to do your tasks when it is the best time for him to touch the DOM and to its DOM work.

For that the browser has mainly two methods that you can use :

requestAnimationFrame : usually used to perform animation but it is the best moment to ask the browser to do DOM operation.
requestIdleCallback : to perform background and low priority work

So here is an example combining the both methods : https://codesandbox.io/s/7jz3yjow0q

It is based on this Google developer article.

And here are some great resources to read about performance on the web :

How to properly optimize a long for loop that concats a long string everytime?

2 Answers2