filtirnig script is too slow

Question

I have the script below to filter unordered list of links (ul, li, a) , the problem is the search is too slow on computer and freeze completlly on mobile, the list is about 19000 link, is it a problem inn the script or is it something else, any ideas?

function myFunction3() {
    var input, filter, ul, li, a, i;
    input = document.getElementById("myInput");
    filter = input.value.toUpperCase();
    ul = document.getElementById("myUL");
    li = ul.getElementsByTagName("li");
    for (i = 0; i < li.length; i++) {
        a = li[i].getElementsByTagName("a")[0];
        if (a.innerHTML.toUpperCase().indexOf(filter) > -1) {
            li[i].style.display = "";
        } else {
            li[i].style.display = "none";

        }
    }
}
</script>

you should use `a.textContent.trim().toUpperCase().includes(filter)`. Though it won't affect the performance AFAIK. — DecPK, Jun 23 '21 at 12:34
`getElementsByTagName` is [a problem](https://stackoverflow.com/a/66480319/1169519) in the loop, and setting inline styles is another. Use array instead of HTMLCollection, and change the class instead of setting the inline style of the elements. Reading `innerHTML` of the live document is also slow, retrieve `textContent` instead. — Teemu, Jun 23 '21 at 12:38
Why are you rendering 19000 list items, that doesn't makes sense. i think that's too much to begin with. initally render limited number of list items, and render remaining list on demand of user. — Gulam Hussain, Jun 23 '21 at 12:44
Like [so](https://jsfiddle.net/knst6cwf/) ... If it's still slow, you can try [RegExp.test](https://developer.mozilla.org/en-US/docs/Web/EXSLT/regexp/test) method instead of checking the index. — Teemu, Jun 23 '21 at 12:45
@MrT [There's another fiddle](https://jsfiddle.net/knst6cwf/1/), it uses a RegExp for filtering. The basic problems are that you're changing the layout, and then read `innerHTML`, which forces a recalculation of the layout, and secondly as it was already said in the linked answer of mine, iterating a live HTMLCollection is really slow. — Teemu, Jun 23 '21 at 12:56
@MrT There was a typo in both of the snippets, the new definition of `for` loops is `for (i = 0, eI = li.length; i < eI; i++)`. I tested the cases. The RegExp code is slightly faster than indexOf code, these are 2 - 3 times faster than Alex' code. We're talking about 50 msecs against 150 msecs, that's negligible. Re-calculation and rendering takes the most of the used time with all of the functions. You might get this faster by using multiple `ul`s, update the lists visible on the screen first, then lazy-update the rest of the lists, An infinite scroll library might also help. — Teemu, Jun 23 '21 at 15:02
It appeared, that FF is extremely slow when creating a list of 19000 items, and also when updating it. Chromium browsers are somewhat ten times faster to create the big list and recalculate and rerender the updates. — Teemu, Jun 23 '21 at 15:31

score 1 · Accepted Answer · answered Jun 23 '21 at 12:39

1

I supose all the li elements are inside your ul element, so you could try something like this:

function myFunction3() {
 var input, filter, ul, li, a, i;
 input = document.getElementById("myInput");
 filter = input.value.toUpperCase();
 ul = document.getElementById("myUL");
 ul.style.display='none'
 li = ul.getElementsByTagName("li");
 for (i = 0; i < li.length; i++) {
    a = li[i].getElementsByTagName("a")[0];
    if (a.innerHTML.toUpperCase().indexOf(filter) > -1) {
        li[i].style.display = "";
    } else {
        li[i].style.display = "none";

    }
 }
 ul.style.display='block'
}

This way the interface will not update visually until the for loop ends, since the parent element will not be visible until the execution ends. I put display = 'block' only as an example. I hope it works.

answered Jun 23 '21 at 12:39

Alex Pinilla

101
3

any more suggestions to enhance it? – Mr T Jun 23 '21 at 12:50
I'm glad it works better for you. About the code, I think it's pretty cool, you could make the code one line smaller and avoid the else if you want by doing this: for (i = 0; i < li.length; i++) { a = li[i].getElementsByTagName("a")[0]; if (a.innerHTML.toUpperCase().indexOf(filter) > -1) { li[i].style.display = ""; continue; } li[i].style.display = "none"; } – Alex Pinilla Jun 23 '21 at 13:04
"_the interface will not update visually until the for loop ends_" This is not quite right, the view is not repainted until all the scripts has been finished anyway. The difference here is, that setting `display: none` pulls the elements out of the textflow, that prevents the layout re-calculation when `a.innerHTML` is retrieved in the loop. – Teemu Jun 24 '21 at 05:17

filtirnig script is too slow

1 Answers1